SWEEP: Replace J2N's TripleShift call with C# 11's unsigned right shift operator#1007
Conversation
|
Need to run better benchmarks to be sure, but this seems to result in about a 7% reduction in time to run all the tests on Linux and macOS. Results are negligible on Windows. |
NightOwl888
left a comment
There was a problem hiding this comment.
I found some places where the extra parentheses were left in, but these don't necessarily need to be fixed. See my comments inline.
I also ran some benchmarks on Windows x64:
IndexFiles
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19045
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=8.0.403
[Host] : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
000-4.8.0-beta00014 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
001-4.8.0-beta00015 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
002-4.8.0-beta00016 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
003-4.8.0-beta00017 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
004-With issue/1006 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
InvocationCount=1 IterationCount=15 LaunchCount=2
UnrollFactor=1 WarmupCount=10
| Method | Job | NuGetReferences | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| IndexFiles | 000-4.8.0-beta00014 | Lucene.Net.Analysis.Common 4.8.0-beta00014 | 721.0 ms | 44.33 ms | 63.57 ms | 44000.0000 | 8000.0000 | 7000.0000 | 220.21 MB |
| IndexFiles | 001-4.8.0-beta00015 | Lucene.Net.Analysis.Common 4.8.0-beta00015 | 698.5 ms | 27.47 ms | 38.51 ms | 44000.0000 | 8000.0000 | 7000.0000 | 220.46 MB |
| IndexFiles | 002-4.8.0-beta00016 | Lucene.Net.Analysis.Common 4.8.0-beta00016 | 688.2 ms | 28.98 ms | 40.62 ms | 44000.0000 | 8000.0000 | 7000.0000 | 220.45 MB |
| IndexFiles | 003-4.8.0-beta00017 | Lucene.Net.Analysis.Common 4.8.0-beta00017 | 601.5 ms | 20.05 ms | 29.39 ms | 42000.0000 | 8000.0000 | 7000.0000 | 211.61 MB |
| IndexFiles | 004-With issue/1006 | Lucene.Net.Analysis.Common 4.8.0-ci0000003030 | 650.0 ms | 39.17 ms | 56.17 ms | 42000.0000 | 8000.0000 | 7000.0000 | 211.64 MB |
Search Files
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19045
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=8.0.403
[Host] : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
000-4.8.0-beta00014 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
001-4.8.0-beta00015 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
002-4.8.0-beta00016 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
003-4.8.0-beta00017 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
004-With issue/1006 : .NET Core 8.0.10 (CoreCLR 8.0.1024.46610, CoreFX 8.0.1024.46610), X64 RyuJIT
IterationCount=15 LaunchCount=2 WarmupCount=10
| Method | Job | NuGetReferences | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| SearchFiles | 000-4.8.0-beta00014 | Lucene.Net.Analysis.Common 4.8.0-beta00014,Lucene.Net.QueryParser 4.8.0-beta00014 | 118.9 ms | 3.16 ms | 4.64 ms | 14000.0000 | 1000.0000 | - | 65.83 MB |
| SearchFiles | 001-4.8.0-beta00015 | Lucene.Net.Analysis.Common 4.8.0-beta00015,Lucene.Net.QueryParser 4.8.0-beta00015 | 139.8 ms | 4.63 ms | 6.79 ms | 14000.0000 | 1000.0000 | - | 65.89 MB |
| SearchFiles | 002-4.8.0-beta00016 | Lucene.Net.Analysis.Common 4.8.0-beta00016,Lucene.Net.QueryParser 4.8.0-beta00016 | 151.3 ms | 13.73 ms | 20.13 ms | 14000.0000 | 1000.0000 | - | 65.89 MB |
| SearchFiles | 003-4.8.0-beta00017 | Lucene.Net.Analysis.Common 4.8.0-beta00017,Lucene.Net.QueryParser 4.8.0-beta00017 | 168.5 ms | 7.64 ms | 11.19 ms | 14000.0000 | 1000.0000 | - | 65.98 MB |
| SearchFiles | 004-With issue/1006 | Lucene.Net.Analysis.Common 4.8.0-ci0000003030,Lucene.Net.QueryParser 4.8.0-ci0000003030 | 158.3 ms | 4.85 ms | 7.10 ms | 14000.0000 | 1000.0000 | - | 65.98 MB |
If you want to run them on other platforms, here is the branch where I added the benchmarkdotnet changes:
https://github.com/NightOwl888/lucenenet/tree/issue/1006-w-benchmarks
Add the config:
new BuildConfiguration { PackageVersion = "4.8.0-ci0000003030", Id = "With issue/1006" },And you can download the nuget packages here: https://dev.azure.com/shad0962/Experiments/_build/results?buildId=2496&view=artifacts&pathAsName=false&type=publishedArtifacts
You will probably also have to add an entry to NuGet.config pointing to your local file system for it to pick up the new version.
Tip: Comment everything out except the new version to make sure it compiles before doing the run, otherwise if the NuGet packages are not found it will simply put N/A after waiting for the entire run to finish.
Updated [Lucene.Net](https://github.com/apache/lucenenet) from 4.8.0-beta00017 to 4.8.0-beta00018. <details> <summary>Release notes</summary> _Sourced from [Lucene.Net's releases](https://github.com/apache/lucenenet/releases)._ ## 4.8.0-beta00018 > This is a maintenance update that upgrades ICU4N to the latest version, since several serious concurrency and resource loading bugs have been patched since the last Lucene.NET release. <!-- Release notes generated using configuration in .github/release.yml at Lucene.Net_4_8_0_beta00018 --> ## What's Changed ### 🐞 Bug Fixes * FuzzyQuery produces a wrong result when prefix is equal to the term length by @paulirwin in apache/lucenenet#1002 * Validate PatternParser DTDs against expected name by @paulirwin in apache/lucenenet#1358 * Validate file paths for FSDirectory and Replicator by @paulirwin in apache/lucenenet#1357 * Bumped ICU4N to 60.1.0-alpha.440 by @NightOwl888 in apache/lucenenet#1353 * ShingleFilter produces invalid queries by @tohidemyname in apache/lucenenet#946 * Fix SegmentInfos replace doesn't update userData by @tohidemyname in apache/lucenenet#948 ### 🚀 Performance Improvements * SWEEP: Replace J2N's TripleShift call with C# 11's unsigned right shift operator by @paulirwin in apache/lucenenet#1007 ### 🏆 Improvements * Added "Improvements" Category for Release Notes by @NightOwl888 in apache/lucenenet#1015 ### 📄 Website and API Documentation * website/site/.htaccess - bug fix by removing BOM and update to beta0017 redirection by @rclabo in apache/lucenenet#1005 * Updated .htaccess copy and release procedure by @NightOwl888 in apache/lucenenet#1010 * Added GitHub Automation for Release Notes by @NightOwl888 in apache/lucenenet#1011 * fix: Render ASF policy links in static HTML footer by @rbowen in apache/lucenenet#1303 * Fix/apidocs breadcrumb toc asf by @zka26 in apache/lucenenet#1232 * README: fix typo MacOS -> macOS by @jbampton in apache/lucenenet#1179 * Added ASF-required links using drop-down menu and unified navigation by @zka26 in apache/lucenenet#1198 * fix: Self-host all external website dependencies by @mmafrar in apache/lucenenet#1197 * Fix typos by @jbampton in apache/lucenenet#1177 * Replace lucene.testSettings.config references with lucene.testsettings.json by @paulirwin in apache/lucenenet#1035 ## New Contributors * @jbampton made their first contribution in apache/lucenenet#1177 * @mmafrar made their first contribution in apache/lucenenet#1197 * @rbowen made their first contribution in apache/lucenenet#1303 * @tohidemyname made their first contribution in apache/lucenenet#946 * @zka26 made their first contribution in apache/lucenenet#1198 **Full Changelog**: apache/lucenenet@Lucene.Net_4_8_0_beta00017...Lucene.Net_4_8_0_beta00018 Commits viewable in [compare view](apache/lucenenet@Lucene.Net_4_8_0_beta00017...Lucene.Net_4_8_0_beta00018). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…596) Updated [Lucene.Net.QueryParser](https://github.com/apache/lucenenet) from 4.8.0-beta00017 to 4.8.0-beta00018. <details> <summary>Release notes</summary> _Sourced from [Lucene.Net.QueryParser's releases](https://github.com/apache/lucenenet/releases)._ ## 4.8.0-beta00018 > This is a maintenance update that upgrades ICU4N to the latest version, since several serious concurrency and resource loading bugs have been patched since the last Lucene.NET release. <!-- Release notes generated using configuration in .github/release.yml at Lucene.Net_4_8_0_beta00018 --> ## What's Changed ### 🐞 Bug Fixes * FuzzyQuery produces a wrong result when prefix is equal to the term length by @paulirwin in apache/lucenenet#1002 * Validate PatternParser DTDs against expected name by @paulirwin in apache/lucenenet#1358 * Validate file paths for FSDirectory and Replicator by @paulirwin in apache/lucenenet#1357 * Bumped ICU4N to 60.1.0-alpha.440 by @NightOwl888 in apache/lucenenet#1353 * ShingleFilter produces invalid queries by @tohidemyname in apache/lucenenet#946 * Fix SegmentInfos replace doesn't update userData by @tohidemyname in apache/lucenenet#948 ### 🚀 Performance Improvements * SWEEP: Replace J2N's TripleShift call with C# 11's unsigned right shift operator by @paulirwin in apache/lucenenet#1007 ### 🏆 Improvements * Added "Improvements" Category for Release Notes by @NightOwl888 in apache/lucenenet#1015 ### 📄 Website and API Documentation * website/site/.htaccess - bug fix by removing BOM and update to beta0017 redirection by @rclabo in apache/lucenenet#1005 * Updated .htaccess copy and release procedure by @NightOwl888 in apache/lucenenet#1010 * Added GitHub Automation for Release Notes by @NightOwl888 in apache/lucenenet#1011 * fix: Render ASF policy links in static HTML footer by @rbowen in apache/lucenenet#1303 * Fix/apidocs breadcrumb toc asf by @zka26 in apache/lucenenet#1232 * README: fix typo MacOS -> macOS by @jbampton in apache/lucenenet#1179 * Added ASF-required links using drop-down menu and unified navigation by @zka26 in apache/lucenenet#1198 * fix: Self-host all external website dependencies by @mmafrar in apache/lucenenet#1197 * Fix typos by @jbampton in apache/lucenenet#1177 * Replace lucene.testSettings.config references with lucene.testsettings.json by @paulirwin in apache/lucenenet#1035 ## New Contributors * @jbampton made their first contribution in apache/lucenenet#1177 * @mmafrar made their first contribution in apache/lucenenet#1197 * @rbowen made their first contribution in apache/lucenenet#1303 * @tohidemyname made their first contribution in apache/lucenenet#946 * @zka26 made their first contribution in apache/lucenenet#1198 **Full Changelog**: apache/lucenenet@Lucene.Net_4_8_0_beta00017...Lucene.Net_4_8_0_beta00018 Commits viewable in [compare view](apache/lucenenet@Lucene.Net_4_8_0_beta00017...Lucene.Net_4_8_0_beta00018). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Replaces calls to J2N's TripleShift method with the C# 11 unsigned right shift operator (
>>>)Fixes #1006
Description
This more closely matches the Java code, and should improve performance by avoiding method calls and reducing this operation down to one opcode.