-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minor regression in System.Tests.Perf_Char.GetUnicodeCategory for non-ascii characters #41107
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
FWIW I just got results from @GrabYourPitchforks and his machine is the only one which is not showing the regression. It's also the only Skylake machine I had access to: System.Tests.Perf_Char.GetUnicodeCategory(c: '?')
|
Direct link to benchmark code (see the |
@GrabYourPitchforks @adamsitnik do you believe this should block 6.0 release at this point? BTW, presumably we haven't fixed it in the meantime? |
@danmoseley Looks like average regression is 1ns, with 3ns being the worst case? It's worth investigating why this regressed, but I wouldn't block release for it. Some really premature speculation: when the original change was checked in, we also did a little bit of size-on-disk work to try to shrink the size of the data tables we carry. Perhaps this microbenchmark shows the result of the extra bit twiddling needed as a result of this size optimization? |
OK moved. |
Possible low hanging fruit optimization: consider having |
I am now running an independent 3.1 vs 5.0 comparison and I've confirmed that it's actual regression introduced in 5.0.
The issue has been originally reported by a bot owned by @DrewScoggins in DrewScoggins/performance-2#574
Repro
System.Tests.Perf_Char.GetUnicodeCategory(c: '?')
No regressions for ascii chars:
System.Tests.Perf_Char.GetUnicodeCategory(c: '.')
System.Tests.Perf_Char.GetUnicodeCategory(c: '.')
@GrabYourPitchforks I am adding 6.0 milestone since it's most probably rare and not used on hot-paths
When you take a look at disassembly, it's clear why it has regressed. I am not pasting it here because I plan to run a workshop next week and would love to get the answer from attendees
The text was updated successfully, but these errors were encountered: