Closed Bug 1361392 Opened 3 years ago Closed 3 years ago

Inefficient memory use in gfxHarfBuzzShaper::ShapeText

Categories

(Core :: Graphics: Text, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jseward, Assigned: jfkthame)

Details

Attachments

(2 files)

Setup, STR, etc, exactly as with bug 1361372.

DHAT says we allocate 9138 blocks via

  gfxHarfBuzzShaper::ShapeText
    hb_buffer_add_utf16 
      hb_buffer_add_utf
        ensure
          hb_buffer_t::enlarge
            realloc
              malloc

always of size 640.  Averaged over all allocations, each byte is written
0.40 times and read only 0.18 times.  From the log (see attachment) there
are no uses at offset 560 or after in any block, and almost no uses after
the halfway point (offset 320).
Attached file DHAT output
This sounds pretty reasonable, actually, given that we're normally shaping an individual (fairly short) word, but harfbuzz can't know this; so it allocates space for glyph details and position data in 32-char increments (IIRC). Usually, we only use the first few characters' worth of that space.

I suppose we could optimize slightly, though, by caching the hb_buffer record in gfxHarfBuzzShaper and just clearing its contents, instead of discarding the buffer and creating a new one for each ShapeText call.
This should eliminate three malloc/free operations (the buffer itself, and its internal 'info' and 'pos' arrays) per ShapeText call, in general.
Attachment #8863794 - Flags: review?(jmuizelaar)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8863794 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/eee394a4575b6caf23b820c930059bf994ea228d
Bug 1361392 - Re-use the hb_buffer in gfxHarfBuzzShaper instead of creating/destroying it on each call to ShapeText. r=jrmuizel
Backout by kwierso@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21a512c16cf8
Backed out changeset eee394a4575b for leaksanitizer failures in pretty much all ASAN mochitests a=backout
Sorry -- I messed up when rebasing the patch to inbound tip. :(  Fixed version coming....
Flags: needinfo?(jfkthame)
https://hg.mozilla.org/integration/mozilla-inbound/rev/22c2c450b41e59198d38aa9936072de43ab77703
Bug 1361392 - Re-use the hb_buffer in gfxHarfBuzzShaper instead of creating/destroying it on each call to ShapeText. r=jrmuizel
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/22c2c450b41e
Re-use the hb_buffer in gfxHarfBuzzShaper instead of creating/destroying it on each call to ShapeText. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/22c2c450b41e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1360881
No longer blocks: 1360881
No longer blocks: 1360878
You need to log in before you can comment on or make changes to this bug.