Bug 101269 – Optimized kerning and ligatures using caching

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Geoffrey Garen]

Optimized kerning and ligatures using caching

[Geoffrey Garen]

Created attachment 172466 [details]
Patch

[Simon Fraser (smfr)]

What's the memory impact of this cache on some common pages?

Comment on attachment 172466 [details]
Patch

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

> Source/WebCore/platform/graphics/WidthCache.h:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

It is still 2012.

> Source/WebCore/platform/graphics/WidthCache.h:8
> + *

Please use the BSD-style license header.

Comment on attachment 172466 [details]
Patch

The rest of the patch looks great! Is Xcode the only build system that needs to be told of the new header file?

[Geoffrey Garen]

> What's the memory impact of this cache on some common pages?

Notwithstanding what seems like a bug in how membuster3 measures "End", this patch looks like no significant change.

Here are the full results from membuster3:

NO patch:

****************************************************************************************************
Run #	   Process	Max		Mean		End
****************************************************************************************************
Run 1:
	    Safari	14.83 MB	10.92 MB	12.39 MB
	WebProcess	578.16 MB	280.24 MB	183.95 MB
	     Total	593.00 MB	291.16 MB	196.34 MB
Run 2:
	    Safari	14.77 MB	11.00 MB	7.10 MB
	WebProcess	571.30 MB	281.36 MB	5.91 MB
	     Total	586.07 MB	292.36 MB	13.02 MB
Run 3:
	    Safari	14.75 MB	10.94 MB	12.36 MB
	WebProcess	571.87 MB	282.98 MB	347.13 MB
	     Total	586.62 MB	293.92 MB	359.49 MB
****************************************************************************************************
Mean (Confidence)	Minimum Run	Statistic
****************************************************************************************************
292.48 MB +/- 0.54%	291.16 MB	Mean Allocations
588.56 MB +/- 0.74%	586.07 MB	Max Allocations
189.62 MB +/- 103.44%	13.02 MB	End Allocations
****************************************************************************************************

YES patch:

****************************************************************************************************
Run #	   Process	Max		Mean		End
****************************************************************************************************
Run 1:
	    Safari	14.61 MB	10.93 MB	12.63 MB
	WebProcess	576.99 MB	283.67 MB	494.06 MB
	     Total	591.59 MB	294.60 MB	506.69 MB
Run 2:
	    Safari	14.59 MB	10.99 MB	7.35 MB
	WebProcess	582.06 MB	282.56 MB	63.48 MB
	     Total	596.66 MB	293.55 MB	70.84 MB
Run 3:
	    Safari	14.49 MB	10.95 MB	10.48 MB
	WebProcess	580.75 MB	281.35 MB	409.96 MB
	     Total	595.23 MB	292.30 MB	420.44 MB
****************************************************************************************************
Mean (Confidence)	Minimum Run	Statistic
****************************************************************************************************
293.48 MB +/- 0.45%	292.30 MB	Mean Allocations
594.49 MB +/- 0.50%	591.59 MB	Max Allocations
332.66 MB +/- 78.51%	70.84 MB	End Allocations
****************************************************************************************************

[Simon Fraser (smfr)]

(In reply to comment #5)
> > What's the memory impact of this cache on some common pages?
> 
> Notwithstanding what seems like a bug in how membuster3 measures "End", this patch looks like no significant change.

Hmm, if I edit that down:
NO patch:
    WebProcess    578.16 MB    280.24 MB    183.95 MB
    WebProcess    571.30 MB    281.36 MB    5.91 MB
    WebProcess    571.87 MB    282.98 MB    347.13 MB

YES patch:
    WebProcess    576.99 MB    283.67 MB    494.06 MB
    WebProcess    582.06 MB    282.56 MB    63.48 MB
    WebProcess    580.75 MB    281.35 MB    409.96 MB

The 'end' results are all higher with the patch.

Can you instrument your cache specifically to find out how much memory it adds?

Simon

[Geoffrey Garen]

> The 'end' results are all higher with the patch.

Something's broken about the 'end' results. They report 100% variance (!). This cache can't effect them because it gets deleted when you close the tab.

> Can you instrument your cache specifically to find out how much memory it adds?

Sure.

[Geoffrey Garen]

Some instrumented statistics on the cache:
apple.com: 5kB
wikipedia.org: 15kB
apple.com/store: 36kB
cnn.com: 36kB
nytimes.com: 81kB
nbcnews.com article: 92kB
nytimes.com article: 99kB
http://en.wikipedia.org/wiki/Main_Page: 120kB
latimes.com article: 213kB
cnn.com article: 220kB
http://en.wikipedia.org/wiki/Steve_jobs: 1MB

The wikipedia article grows to a 1MB cache because it performs 16000 layouts during load. Without the cache, those layouts spend 699ms kerning and legating 251195 text runs; with it, they spend 260ms kerning and legating 82605 text runs. I believe the speed is worth the memory in this case. (Of course, the real answer is that we shouldn't layout wikipedia pages 16000 times. I'll file a bug about that.)

[Geoffrey Garen]

> > Source/WebCore/platform/graphics/WidthCache.h:2
> > + * Copyright (C) 2013 Apple Inc. All rights reserved.
> 
> It is still 2012.

I accept your optimism about when this patch will be finished ;).

> > Source/WebCore/platform/graphics/WidthCache.h:8
> > + *
> 
> Please use the BSD-style license header.

Will do.

[Eric Seidel (no email)]

Please CC me on the 16k layout bug. :)

[Geoffrey Garen]

Created attachment 172674 [details]
Patch

[Stephanie Lewis]

I broke end results.  There's a patch out to fix it.  Based on the max number which is accurate but is usually more varient I would be concerned there is a small regression.

Comment on attachment 172674 [details]
Patch

What’s up with the EWS bots?

<rdar://problem/12329057>

[Eric Seidel (no email)]

They seem to have had trouble applying your project diff:

Hunk #4 FAILED at 25676.
1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/WebCore.xcodeproj/project.pbxproj.rej

[Geoffrey Garen]

Committed r133921: <http://trac.webkit.org/changeset/133921>