You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/03/15 14:31:03 UTC
[GitHub] [lucenenet] NightOwl888 commented on issue #429: FieldDoc.Fields boxing issue
NightOwl888 commented on issue #429:
URL: https://github.com/apache/lucenenet/issues/429#issuecomment-799466687
So, @rclabo and I have been analyzing this in more detail and it is clear that `FieldDoc.Fields` cannot be made generic because it contains a mixed bag of field types.
As an alternative, some design proposals to `FieldComparer<T>` have been benchmarked in [BenchmarkFieldComparer](https://github.com/NightOwl888/BenchmarkFieldComparer) to see what the impact of such changes might be.
## Benchmarks
### Cast to Number
<details>
<summary>Click to expand!</summary>
``` ini
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.104
[Host] : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
Job-TNEQUD : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
InvocationCount=1 UnrollFactor=1
```
| Method | Mean | Error | StdDev | Median | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------------------- |---------:|---------:|---------:|---------:|------------:|------:|------:|--------------:|
| CastComparer_PureInt32 | 207.1 ms | 4.13 ms | 9.65 ms | 211.2 ms | - | - | - | 3.93 KB |
| CastComparer_PureFloat | 210.7 ms | 4.18 ms | 11.02 ms | 213.2 ms | - | - | - | 3.93 KB |
| ValueType_CastToNumber_Int32 | 717.3 ms | 14.15 ms | 31.35 ms | 715.8 ms | 510000.0000 | - | - | 2343753.93 KB |
| ValueType_CastToNumber_Float | 770.8 ms | 15.25 ms | 22.82 ms | 772.6 ms | 510000.0000 | - | - | 2343753.93 KB |
| ReferenceType_CastToNumber_Int32_CastComparer | 309.5 ms | 6.18 ms | 14.93 ms | 310.2 ms | - | - | - | 3.93 KB |
| ReferenceType_CastToNumber_Float_CastComparer | 360.4 ms | 7.13 ms | 19.65 ms | 364.4 ms | - | - | - | 3.93 KB |
| ReferenceType_CastToNumber_Int32_IConvertible | 631.2 ms | 12.03 ms | 23.46 ms | 627.7 ms | - | - | - | 3.93 KB |
| ReferenceType_CastToNumber_Float_IConvertible | 764.5 ms | 15.23 ms | 38.20 ms | 768.6 ms | - | - | - | 3.93 KB |
</details>
### Copy
<details>
<summary>Click to expand!</summary>
``` ini
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.104
[Host] : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
Job-CBNWUR : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
InvocationCount=1 UnrollFactor=1
```
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------- |---------:|--------:|--------:|------:|------:|------:|----------:|
| ValueType_Copy | 104.3 ms | 2.51 ms | 7.37 ms | - | - | - | - |
| ReferenceType_Copy | 136.1 ms | 2.70 ms | 6.68 ms | - | - | - | - |
</details>
### Fill Fields
<details>
<summary>Click to expand!</summary>
``` ini
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.104
[Host] : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
Job-TNEQUD : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
InvocationCount=1 UnrollFactor=1
```
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------------- |---------:|--------:|---------:|------:|------:|------:|----------:|
| ValueType_FillFields | 253.1 us | 9.99 us | 27.34 us | - | - | - | 480480 B |
| ReferenceType_FillFields | 175.8 us | 3.31 us | 3.68 us | - | - | - | 480 B |
</details>
### Set Top Value
<details>
<summary>Click to expand!</summary>
``` ini
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.104
[Host] : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
Job-ZELLKO : .NET Core 3.1.13 (CoreCLR 4.700.21.11102, CoreFX 4.700.21.11602), X64 RyuJIT
InvocationCount=1 UnrollFactor=1
```
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|-------------------------- |---------:|----------:|----------:|------:|------:|------:|----------:|
| ValueType_SetTopValue | 1.391 ms | 0.0969 ms | 0.2652 ms | - | - | - | - |
| ReferenceType_SetTopValue | 2.474 ms | 0.0602 ms | 0.1765 ms | - | - | - | - |
</details>
Clearly, allocations used in the [`FieldValueHitQueue<T>.FillFields()`](https://github.com/apache/lucenenet/blob/2731bfd4443cb12af664c03d0683b938884f1448/src/Lucene.Net/Search/FieldValueHitQueue.cs#L233-L243) method and when moving the numeric data out of the fields can be dramatically reduced.
The leading contender, [BoxingWrappedReferenceScenario](https://github.com/NightOwl888/BenchmarkFieldComparer/blob/master/BenchmarkFieldComparer/BoxingWrappedReferenceScenario.cs) takes us a step back toward the Lucene design and although the details are not worked out, here are some of the basics:
1. `FieldComparer<T>` gets a generic constraint to disallow value types.
2. `NumericComparer<T>` is changed to `NumericComparer<TValue, TWrapper>`. `TWrapper` is the type that is used in its base class `FieldComparer<T>` and `TValue` is the value type that is exposed on the public API.
3. A series of reference type wrappers are made for each numeric type similar to the [Numbers Classes](https://docs.oracle.com/javase/tutorial/java/data/numberclasses.html) in Java. However, unlike our previous one-off approach to building wrappers whenever reference types are required these will be fully implemented with full tests, and put into `J2N`.
- These classes will have all of the same interfaces that numeric types have in .NET including `IConverible` and `IComparable<T>`.
- To reduce the number of breaking changes introduced to users, they will also use the [implicit operator](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/user-defined-conversion-operators) to convert the wrapped value to the numeric type without a cast.
4. The number class references are used as the `TWrapper` type in `NumericComparer<TValue, TWrapper>`.
5. Other related systems, such as the field cache may also be refactored to both enforce the use of reference types and to allow the system to pass the numbers class instances all the way through from their creation in `Field` to the `FillFields()` method without having to unwrap and rewrap the numeric value in a reference type.
Of course, none of this is set in stone, but it is clear that fixing this problem will likely involve breaking some public APIs at least a little so I am moving this to the `4.8.0-beta00015` milestone.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org