See this line:
|
ulong sortableBits = BitConverter.ToUInt64(BitConverter.GetBytes(val), 0) ^ 0x8000000000000000L; // LUCENENET TODO: Performance - Benchmark this |
This was found during code review to do an unnecessary byte[] allocation and memory read, when the upstream Java code is just:
https://github.com/apache/lucene/blob/f01152a5909fa6059f4f1d4aeb4e14968ef1d8c2/lucene/core/src/java/org/apache/lucene/util/NumericUtils.java#L146
I'm guessing this was due to that XOR'ed literal being outside the range of C#'s long, instead of wrapping around to long's min value like it does in Java, combined with unsigned right shift not being available at the time. We should be able to replace this with:
long sortableBits = val ^ long.MinValue;
... and the shifts below it to use the unsigned right shift assignment operator (>>>=) to match upstream.
Per the comment, this change should be benchmarked to see what the impact is, but removing the array allocation and memory read should be significant.
Filing this issue to track it, but I've already got a prototype locally and will PR this myself. Initial microbenchmark results of this method are 2-3x faster than beta 17, but I think there's even room to go further.
See this line:
lucenenet/src/Lucene.Net/Util/NumericUtils.cs
Line 163 in 9a1bf2e
This was found during code review to do an unnecessary
byte[]allocation and memory read, when the upstream Java code is just:https://github.com/apache/lucene/blob/f01152a5909fa6059f4f1d4aeb4e14968ef1d8c2/lucene/core/src/java/org/apache/lucene/util/NumericUtils.java#L146
I'm guessing this was due to that XOR'ed literal being outside the range of C#'s long, instead of wrapping around to long's min value like it does in Java, combined with unsigned right shift not being available at the time. We should be able to replace this with:
... and the shifts below it to use the unsigned right shift assignment operator (
>>>=) to match upstream.Per the comment, this change should be benchmarked to see what the impact is, but removing the array allocation and memory read should be significant.
Filing this issue to track it, but I've already got a prototype locally and will PR this myself. Initial microbenchmark results of this method are 2-3x faster than beta 17, but I think there's even room to go further.