[webkit-reviews] review granted: [Bug 46374] [Chromium] FontLinux performance improvement and cleanup : [Attachment 68789] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Sep 24 17:39:47 PDT 2010
David Levin <levin at chromium.org> has granted Xiaomei Ji <xji at chromium.org>'s
request for review:
Bug 46374: [Chromium] FontLinux performance improvement and cleanup
https://bugs.webkit.org/show_bug.cgi?id=46374
Attachment 68789: patch
https://bugs.webkit.org/attachment.cgi?id=68789&action=review
------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68789&action=review
Please consider adjusting the ChangeLog.
> WebCore/ChangeLog:46
> + and new glyph arrays so the arrays in use are clean initially.
> + But it introduced more delete/new operation which might be a problem
in
> + memory tight devices (such as Android device).
> +
> + This fix proposed to
> + 1. resume the setting of
> + m_item.num_glyphs = m_maxGlyphs before HB_ShapeItem() call to
minimize
> + the number of delete/new operations
> + 2. always cleanup the glyph arrays before re-use them to make sure
> + the arrays in use are clean initially.
> +
> + Using http://www.google.ae/preferences?hl=ar as an example.
> +
> + Following is the change of m_item.num_glyphs after the call of
HB_ShapeItem().
> +
> + 54 --> 5
> + 5 --> 1
> + 1 --> 8
> + .....
> +
> + The array is zero-ed initially (with size 54), so there is no
problem
> + when shaping the first script run. After shaping, the
m_item.num_glyphs
> + changed to 5.
>
> + Then, when shaping next script run, since there is enough space
available
> + for HB_ShapeItem(), no deleteGlypyArrays/createGlyphArrays will be
called,
> + but zero-ing array is not called either, so the next script run's
> + shaping is based on a dirty array. This is 2nd issue mentioned above
that
> + the patch addresses.
> +
> + In the 3rd script run, although there is actually an array of size
54,
> + the recorded num_glyphs is 1, and it is less than the needed size,
so,
> + array of size 54 is deleted and an array of size 8 is created.
> + The increase of num_glyphs from one run to its next is not uncommon.
> + It is hit 1096 times during loading the above example page. So the
> + delete/new will introduce extra overhead. This is the 1st issue
mentioned
> + above that the patch addresses.
> +
This is a bit verbose.
How about this:
Reduce new/delete operations by storing the maximum capacity of the glyph array
and use value that
in subsequent HB_ShapeItem calls. (Note that a call to HB_ShapeItem may reduce
the value of m_item.num_glyphs
below the capacity.)
Also be consistent with zero'ing the glyph arrays before calling HB_ShapeItem.
More information about the webkit-reviews
mailing list