extend grapheme_category fast path - #179
Conversation
Based on observations of UCD data, inlined other ranges that can be easily derived at runtime, in addition to ASCII. - `[0x30A0, 0xA660)`: CJK...Vai. All `Any`, except `U+3297`/`U+3299` (`Extended_Pictographic`) - `[0xAC00, 0xD7A4)`: Hangul syllables, `LV` every 28th and `LVT` otherwise - `[0xFE00, 0xFE10)`: Variation selectors, all `Extend` Inlining the evaluation of these ranges allows us to omit ranges from the data in `tables.rs`. The largest range is Hangul syllables, which corresponds to 599 table entries and 16,173 code points, representing 53% of the existing `grapheme_cat_table`. It can be replaced by a single O(1) modulo operation, which is beneficial to other ranges as well, as it reduces the number of comparisons in the worst-case table search by one. Although there is a slight overhead because inline ranges are always evaluated, this loss is offset by an improved cache hit rate, as they are excluded from the range cache. There are a few more derivable ranges, but they are less effective.
64c8c97 to
6ead862
Compare
|
I relocated the ASCII path to the same location. Since Unicode range assumptions are checked in |
| if (ch as u32) < self.grapheme_cat_cache.0 || (ch as u32) > self.grapheme_cat_cache.1 { | ||
| self.grapheme_cat_cache = gr::grapheme_category(ch); | ||
|
|
||
| match gr::grapheme_category(ch) { |
There was a problem hiding this comment.
Changing the codegen makes the changes look a bit inflated, but the key is a single change to the grapheme_category function integration here.
|
This is a pretty large diff, I do not think I will have time to review this any time soon. If you can get another maintainer or a trusted community member to review it I could perhaps perform a faster review, but at the moment I'm probably going to heavily deprioritize this. |
|
Hmm. If I exclude the safety check in the Python code, the diff to review can be significantly reduced. That part isn't really mandatory if the existing Unicode range doesn't change. |
|
I think the consistency check is necessary: we need to be able to make sure things are handled on updates. |
Context: #177
Based on observations of UCD data, inlined other ranges that can be easily derived at runtime, in addition to ASCII.
[0x30A0, 0xA660): CJK...Vai. AllAny, exceptU+3297/U+3299(Extended_Pictographic)[0xAC00, 0xD7A4): Hangul syllables,LVevery 28th andLVTotherwise[0xFE00, 0xFE10): Variation selectors, allExtendInlining the evaluation of these ranges allows us to omit ranges from the data in
tables.rs.The largest range is Hangul syllables, which corresponds to 599 table entries and 16,173 code points, representing 53% of the existing
grapheme_cat_table.It can be replaced by a single O(1) modulo operation, which is beneficial to other ranges as well, as it reduces the number of comparisons in the worst-case table search by one.
Although there is a slight overhead because inline ranges are always evaluated, this loss is offset by an improved cache hit rate, as they are excluded from the range cache.
There are a few more derivable ranges, but they are less effective.