Remove the Glyph.matchesForCache method (PR 13494 follow-up)
#15586
Conversation
Snuffleupagus
changed the title
Glyph.matchesForCache method (PR 13494 follow-up)Glyph.matchesForCache method (PR 13494 follow-up)
Snuffleupagus
force-pushed
the
rm-matchesForCache
branch
3 times, most recently
from
f08442b to
3748c6e
Compare
This method, and its class, was originally added in PR 4453 to reduce memory usage when parsing text. Then PR 13494 extended the `Glyph`-representation slightly to also include the `charCode`, which made the `matchesForCache` method *effectively* redundant since most properties on a `Glyph`-instance indirectly depends on that one. The only exception is potentially `isSpace` in multi-byte strings. Also, something that I noticed when testing this code: The `matchesForCache` method never worked correctly for `Glyph`s containing `accent`-data, since Objects are passed by reference in JavaScript. For affected fonts, of which there's only a handful of examples in our test-suite, we'd fail to find an already existing `Glyph` because of this.
With the changes in the previous patch we can move the glyph-cache lookup to the top of the method and thus avoid a bunch of, in *almost* every case, completely unnecessary re-parsing for every `charCode`.
Snuffleupagus
force-pushed
the
rm-matchesForCache
branch
from
8b68f2c to
fd35cda
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.193.163.58:8877/d26682143e29a14/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/61d17a8ff4c9c07/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/61d17a8ff4c9c07/output.txt Total script time: 24.83 mins
Image differences available at: http://54.241.84.105:8877/61d17a8ff4c9c07/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d26682143e29a14/output.txt Total script time: 28.58 mins
Image differences available at: http://54.193.163.58:8877/d26682143e29a14/reftest-analyzer.html#web=eq.log |
|
Note that being able to skip re-parsing this data over and over for every single rendered glyph is a small performance improvement. Some very quick |
This method, and its class, was originally added in PR #4453 to reduce memory usage when parsing text. Then PR #13494 extended the
Glyph-representation slightly to also include thecharCode, which made thematchesForCachemethod effectively redundant since most properties on aGlyph-instance indirectly depends on that one. The only exception is potentiallyisSpacein multi-byte strings.Also, something that I noticed when testing this code: The
matchesForCachemethod never worked correctly forGlyphs containingaccent-data, since Objects are passed by reference in JavaScript. For affected fonts, of which there's only a handful of examples in our test-suite, we'd fail to find an already existingGlyphbecause of this.