Add tighter bound to range check for matching Regex char classes #67133
Conversation
When we emit a bitmap lookup for character classes containing only ASCII characters, we currently bound the check by 128, e.g. ```C# if (ch < 128 && lookupTable[...]) ``` but we can easily lower that 128 to instead be the actual exclusive upper bound based on the char set. Doing so means we don't need to hit the lookup table for a larger set of characters. (We could also actually shrink the size of the lookup table itself, but doing so would only save a few bytes, and it didn't seem worth the complexity right now. We could also add a lower range check, but that's also additional checks to execute whereas this one is just improving an existing check that's also required for correctness.)
stephentoub
added
area-System.Text.RegularExpressions
tenet-performance
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue DetailsWhen we emit a bitmap lookup for character classes containing only ASCII characters, we currently bound the check by 128, e.g. if (ch < 128 && lookupTable[...])but we can easily lower that 128 to instead be the actual exclusive upper bound based on the char set. Doing so means we don't need to hit the lookup table for a larger set of characters. (We could also actually shrink the size of the lookup table itself, but doing so would only save a few bytes, and it didn't seem worth the complexity right now. We could also add a lower range check, but that's also additional checks to execute whereas this one is just improving an existing check that's also required for correctness.) Contributes to #67056
|
…net#67133) When we emit a bitmap lookup for character classes containing only ASCII characters, we currently bound the check by 128, e.g. ```C# if (ch < 128 && lookupTable[...]) ``` but we can easily lower that 128 to instead be the actual exclusive upper bound based on the char set. Doing so means we don't need to hit the lookup table for a larger set of characters. (We could also actually shrink the size of the lookup table itself, but doing so would only save a few bytes, and it didn't seem worth the complexity right now. We could also add a lower range check, but that's also additional checks to execute whereas this one is just improving an existing check that's also required for correctness.)
ghost
locked as resolved and limited conversation to collaborators
|
Similar regression report here (ubuntu x64, 3/25), looks like whatever it was got fixed subsequently. |
When we emit a bitmap lookup for character classes containing only ASCII characters, we currently bound the check by 128, e.g.
but we can easily lower that 128 to instead be the actual exclusive upper bound based on the char set. Doing so means we don't need to hit the lookup table for a larger set of characters.
(We could also actually shrink the size of the lookup table itself, but doing so would only save a few bytes, and it didn't seem worth the complexity right now. We could also add a lower range check, but that's also additional checks to execute whereas this one is just improving an existing check that's also required for correctness.)
Contributes to #67056