Bug 270492 – [IFC] Let's try to reuse previously computed breaking positions

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

ssia

[zalan]

Created attachment 470168 [details]
Patch

[zalan]

Created attachment 470254 [details]
Patch

[zalan]

Created attachment 470262 [details]
Patch

[zalan]

Created attachment 470272 [details]
Patch

[zalan]

Created attachment 470276 [details]
Patch

[zalan]

Created attachment 470278 [details]
Patch

[zalan]

Created attachment 470281 [details]
Patch

[zalan]

Created attachment 470296 [details]
Patch

[Antti Koivisto]

Comment on attachment 470296 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=470296&action=review

> Source/WebCore/layout/formattingContexts/inline/text/TextBreakingPositionCache.h:80
> +} // namespace Layout
> +} // namespace WebCore
> +
> +namespace WTF {
> +
> +template<> struct HashTraits<WebCore::Layout::TextBreakingPositionContext> : GenericHashTraits<WebCore::Layout::TextBreakingPositionContext> {
> +    static void constructDeletedValue(WebCore::Layout::TextBreakingPositionContext& slot) { slot.isHashTableDeletedValue = true; }
> +    static bool isDeletedValue(const WebCore::Layout::TextBreakingPositionContext& value) { return value.isHashTableDeletedValue; }
> +    static WebCore::Layout::TextBreakingPositionContext emptyValue() { return { }; }
> +};
> +
> +template<> struct DefaultHash<WebCore::Layout::TextBreakingPositionContext> : WebCore::Layout::TextBreakingPositionContextHash { };
> +
> +} // namespace WTF
> +
> +namespace WebCore {
> +namespace Layout {

maybe put the HashTraits goo the the end of the file to avoid bouncing between namespaces?

> Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:1127
> +    Layout::TextBreakingPositionCache::singleton().clear();

It would bet better to call this directly from MemoryRelease.cpp since now it gets unnecessarily called repeatedly for each LineLayout

[zalan]

(In reply to Antti Koivisto from comment #9)
> Comment on attachment 470296 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=470296&action=review
> 
> > Source/WebCore/layout/formattingContexts/inline/text/TextBreakingPositionCache.h:80
> > +} // namespace Layout
> > +} // namespace WebCore
> > +
> > +namespace WTF {
> > +
> > +template<> struct HashTraits<WebCore::Layout::TextBreakingPositionContext> : GenericHashTraits<WebCore::Layout::TextBreakingPositionContext> {
> > +    static void constructDeletedValue(WebCore::Layout::TextBreakingPositionContext& slot) { slot.isHashTableDeletedValue = true; }
> > +    static bool isDeletedValue(const WebCore::Layout::TextBreakingPositionContext& value) { return value.isHashTableDeletedValue; }
> > +    static WebCore::Layout::TextBreakingPositionContext emptyValue() { return { }; }
> > +};
> > +
> > +template<> struct DefaultHash<WebCore::Layout::TextBreakingPositionContext> : WebCore::Layout::TextBreakingPositionContextHash { };
> > +
> > +} // namespace WTF
> > +
> > +namespace WebCore {
> > +namespace Layout {
> 
> maybe put the HashTraits goo the the end of the file to avoid bouncing
> between namespaces?
moved all this to a dedicated file.

> 
> > Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:1127
> > +    Layout::TextBreakingPositionCache::singleton().clear();
> 
> It would bet better to call this directly from MemoryRelease.cpp since now
> it gets unnecessarily called repeatedly for each LineLayout
that's a good point!

[zalan]

Created attachment 470298 [details]
[fast-cq]Patch

[Radar WebKit Bug Importer]

<rdar://problem/124417278>

[EWS]

Committed 275949@main (33c8b3c4ff39): <https://commits.webkit.org/275949@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 470298 [details].