New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2014
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

getStartPositionOfChar and getRotationOfChar run slowly compared to IE

Project Member Reported by jakearchibald@chromium.org, Aug 2 2013

Issue description

http://jsbin.com/imajow/1/quiet (warning: test takes ~20 seconds)

The test looks at some SVG text on a path and recreates it using text elements for each character. This gets bottlenecked on getStartPositionOfChar and getRotationOfChar, which take 8-10ms per call.

Feels like character position is being recalculated per call and could be better cached, or more positions gathered on the first call (at a guess, I think it'd be common to get multiple char positions from the same element).

Firefox & WebKit are similarly slow/slower, IE11 platform preview completes the test in under 500ms, it's doing something smarter.
 
Owner: schenney@chromium.org
Status: Assigned
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 25 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=176937

------------------------------------------------------------------
r176937 | fs@opera.com | 2014-06-25T17:09:09.795827Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/SVGTextQuery.cpp?r1=176937&r2=176936&pathrev=176937

Optimize SVGTextQuery's character offset ligature adjustment query

For queries that take a code unit to compute some property for (basically
all *OfChar calls), the <start, end> range is mapped to the closest
glyph-boundary to allow queries within ligatures.
This adjustment step is O(N), N=number of glyphs within a text node, and
is performed for each fragment within a text box.
This means that for text boxes with a large number of fragments - and,
consequently, a large number of glyphs - there'll be O(N^2) behavior.
Since the <start, end> range is invariant for most queries, it should
suffice to compute the adjusted range once per text node [1].
The easiest way to achieve this is by adding a simple cache and reuse the
result of an adjustment query (for every fragment in a text box.)

This improves the runtime of the test in the bug from ~13s to ~120ms (or
approximately a factor 100) locally.

[1] This CL however only perform caching per text box in order to minimize
modifications to the actual adjustment code.

BUG= 267504 

Review URL: https://codereview.chromium.org/349223003
-----------------------------------------------------------------
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=177148

------------------------------------------------------------------
r177148 | fs@opera.com | 2014-06-27T22:41:56.103502Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/SVGTextQuery.cpp?r1=177148&r2=177147&pathrev=177148

Improve rejection test for character-based SVG text queries

Perform an intersection of the involved ranges to reject fragments early,
while still avoiding calls to mapStartEndPositionsIntoFragmentCoordinates.
This means the "cache" added in https://codereview.chromium.org/349223003
can be dropped, since we'll now never even reach it. This gives a moderate
speed-up - a few percent - on the original TC compared to using the cache.

BUG= 267504 

Review URL: https://codereview.chromium.org/359783002
-----------------------------------------------------------------
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 30 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=177231

------------------------------------------------------------------
r177231 | fs@opera.com | 2014-06-30T18:04:08.202952Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/SVGTextQuery.cpp?r1=177231&r2=177230&pathrev=177231
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/SVGTextQuery.h?r1=177231&r2=177230&pathrev=177231

Simplify SVGTextQuery::modifyStartEndPositionsRespectingLigatures

Simplify this method by passing it the fragment in question, and then use
SVGTextFragment::metricsListOffset to get a reasonable starting point.
Rewrite the big loop into two simpler loops - one for each end-point.
This shaves another ~15% off the TC in the bug (on top of [1]).

[1] https://codereview.chromium.org/359783002/

BUG= 267504 

Review URL: https://codereview.chromium.org/355373003
-----------------------------------------------------------------

Comment 5 by f...@opera.com, Jul 1 2014

Owner: f...@opera.com
Status: Fixed
I have this down to just below 90ms on my machine now (from 16+ seconds). The top on the profile is still within SVGTextQuery, but it's down to walking and rejecting the fragments. Various code-shuffling got me down to ~75ms, but that involved refactoring to make various methods static (which in turn I believe helped inlining-decisions). I might land that at a later stage, but I'll consider this 'fixed' for now.

Sign in to add a comment