Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revert FieldCacheImpl delegate capture (Fixes #370, Closes #371) #389

Merged
merged 2 commits into from
Nov 12, 2020
Merged

Revert FieldCacheImpl delegate capture (Fixes #370, Closes #371) #389

merged 2 commits into from
Nov 12, 2020

Conversation

NightOwl888
Copy link
Contributor

As was discussed in #371, we are definitely seeing a negative impact with this change that was introduced in #348 with commit 34758c1. This PR reverts that commit and removes the unnecessary fields from the "anonymous" classes.

Benchmarks

IndexFiles

Click to expand 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.572 (2004/?/20H1)
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.1.403
  [Host]             : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00005    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00006    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00007    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00008    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00009    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00010    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00011    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00012    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-wip09_before : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-wip10_after  : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), 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 4.8.0-beta00005 Lucene.Net.Analysis.Common 4.8.0-beta00005 628.7 ms 8.56 ms 12.55 ms 43000.0000 8000.0000 7000.0000 220.78 MB
IndexFiles 4.8.0-beta00006 Lucene.Net.Analysis.Common 4.8.0-beta00006 614.4 ms 8.24 ms 11.28 ms 43000.0000 8000.0000 7000.0000 220.73 MB
IndexFiles 4.8.0-beta00007 Lucene.Net.Analysis.Common 4.8.0-beta00007 627.2 ms 12.90 ms 18.51 ms 44000.0000 8000.0000 7000.0000 220.65 MB
IndexFiles 4.8.0-beta00008 Lucene.Net.Analysis.Common 4.8.0-beta00008 603.9 ms 8.15 ms 11.42 ms 43000.0000 8000.0000 7000.0000 220.96 MB
IndexFiles 4.8.0-beta00009 Lucene.Net.Analysis.Common 4.8.0-beta00009 623.6 ms 11.10 ms 15.92 ms 44000.0000 8000.0000 7000.0000 220.92 MB
IndexFiles 4.8.0-beta00010 Lucene.Net.Analysis.Common 4.8.0-beta00010 612.4 ms 11.47 ms 16.07 ms 44000.0000 8000.0000 7000.0000 220.92 MB
IndexFiles 4.8.0-beta00011 Lucene.Net.Analysis.Common 4.8.0-beta00011 622.0 ms 13.63 ms 20.40 ms 43000.0000 8000.0000 7000.0000 220.96 MB
IndexFiles 4.8.0-beta00012 Lucene.Net.Analysis.Common 4.8.0-beta00012 659.4 ms 13.05 ms 19.13 ms 56000.0000 7000.0000 6000.0000 286.57 MB
IndexFiles 4.8.0-wip09_before Lucene.Net.Analysis.Common 4.8.0-ci0000002232 705.6 ms 53.68 ms 80.35 ms 43000.0000 8000.0000 7000.0000 219.96 MB
IndexFiles 4.8.0-wip10_after Lucene.Net.Analysis.Common 4.8.0-ci0000002236 637.8 ms 12.61 ms 18.48 ms 43000.0000 8000.0000 7000.0000 219.82 MB

SearchFiles

Click to expand 
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.572 (2004/?/20H1)
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.1.403
  [Host]             : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00005    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00006    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00007    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00008    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00009    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00010    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00011    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-beta00012    : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-wip09_before : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT
  4.8.0-wip10_after  : .NET Core 3.1.9 (CoreCLR 4.700.20.47201, CoreFX 4.700.20.47203), X64 RyuJIT

IterationCount=15  LaunchCount=2  WarmupCount=10
Method Job NuGetReferences Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
SearchFiles 4.8.0-beta00005 Lucene.Net.Analysis.Common 4.8.0-beta00005,Lucene.Net.QueryParser 4.8.0-beta00005 202.4 ms 7.06 ms 10.57 ms 13000.0000 1000.0000 - 61.74 MB
SearchFiles 4.8.0-beta00006 Lucene.Net.Analysis.Common 4.8.0-beta00006,Lucene.Net.QueryParser 4.8.0-beta00006 207.9 ms 7.74 ms 11.59 ms 13000.0000 1000.0000 - 61.74 MB
SearchFiles 4.8.0-beta00007 Lucene.Net.Analysis.Common 4.8.0-beta00007,Lucene.Net.QueryParser 4.8.0-beta00007 203.9 ms 7.58 ms 11.34 ms 13000.0000 1000.0000 - 61.58 MB
SearchFiles 4.8.0-beta00008 Lucene.Net.Analysis.Common 4.8.0-beta00008,Lucene.Net.QueryParser 4.8.0-beta00008 120.4 ms 1.22 ms 1.82 ms 13000.0000 - - 60.24 MB
SearchFiles 4.8.0-beta00009 Lucene.Net.Analysis.Common 4.8.0-beta00009,Lucene.Net.QueryParser 4.8.0-beta00009 118.0 ms 4.73 ms 7.08 ms 13000.0000 200.0000 - 60.24 MB
SearchFiles 4.8.0-beta00010 Lucene.Net.Analysis.Common 4.8.0-beta00010,Lucene.Net.QueryParser 4.8.0-beta00010 121.8 ms 5.55 ms 8.30 ms 13000.0000 - - 60.03 MB
SearchFiles 4.8.0-beta00011 Lucene.Net.Analysis.Common 4.8.0-beta00011,Lucene.Net.QueryParser 4.8.0-beta00011 118.0 ms 2.51 ms 3.61 ms 13000.0000 - - 60.03 MB
SearchFiles 4.8.0-beta00012 Lucene.Net.Analysis.Common 4.8.0-beta00012,Lucene.Net.QueryParser 4.8.0-beta00012 117.0 ms 1.95 ms 2.74 ms 13200.0000 - - 60.98 MB
SearchFiles 4.8.0-wip09_before Lucene.Net.Analysis.Common 4.8.0-ci0000002232,Lucene.Net.QueryParser 4.8.0-ci0000002232 114.5 ms 3.62 ms 5.31 ms 10800.0000 200.0000 - 49.46 MB
SearchFiles 4.8.0-wip10_after Lucene.Net.Analysis.Common 4.8.0-ci0000002236,Lucene.Net.QueryParser 4.8.0-ci0000002236 112.3 ms 1.58 ms 2.26 ms 10800.0000 200.0000 - 49.46 MB

This just uses inheritance on the numeric fields like we did originally and ignores the fact that there is a constructor overload that uses a delegate (which is convenient, but a bit slower).

The number of searches has been increased from 1000 to 1500, which is why these numbers are higher than prior benchmarks.

@NightOwl888
Revert "Lucene.Net.Search.FieldCacheImpl: Removed field value types a…
b6581fa 
…nd declared Get methods inline using delegate methods"

This reverts commit 34758c1.

As per apache#370, using delegates creates extra allocations so we trade some maintainability for top performance in this case.
@NightOwl888
Lucene.Net.Search.FieldCacheImpl: Removed unused outerInstance fields…
c2f9ec7 
… and marked fields readonly, as allowed
@NightOwl888 NightOwl888 mentioned this pull request Nov 10, 2020
Closed
@NightOwl888 NightOwl888 merged commit 1a02f66 into apache:master Nov 12, 2020
@NightOwl888 NightOwl888 mentioned this pull request Nov 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.8.0-beta00013
Development

Successfully merging this pull request may close these issues.
1 participant
@NightOwl888