NightOwl888 commented on pull request #410: URL: https://github.com/apache/lucenenet/pull/410#issuecomment-779731240
@eladmarg Given the fact that these locks exist in Lucene and this is not a reasonable locking alternative, we should keep them in place here. I would suggest holding off on this until we investigate https://github.com/apache/lucenenet/issues/417 further, as it is still up in the air whether we should just use `Lazy<T>` or build our own wrapper around `LazyInitializer`. Once we have that solution (or solutions) worked out, we can apply it to `FacetConfig` as well. Your last commit, https://github.com/apache/lucenenet/pull/410/commits/0f1894540dd877b47db4a0b86e3ec93cdf2906f4 does contain some useful removal of casts except for 1 call to `Math.Abs`. On [this line](https://github.com/apache/lucenenet/pull/410/commits/0f1894540dd877b47db4a0b86e3ec93cdf2906f4#diff-10b89dfb9b661aa00c8521f46673d331f50fe6ec82a342af7bd58ca7f8b21defR53), this is a bit twiddle that is equivalent to Java's `>>>` operator (unsigned right shift). The cast was done to make it unsigned, however, there is a faster approach that doesn't require a cast (at least not with `int`) is to use [`TripleShift`](https://github.com/NightOwl888/J2N/blob/b24a28da203b55aba860f16d955ce3b1788fcd42/src/J2N/Numerics/BitOperation.cs#L531-L645). I removed more than 1000 casts last night and have passing tests, but I would like to run some benchmarks to see how removing them compares with the casts in place before submitting the PR. Benchmarking the `TripleShift(this int number, int bits)` method against the two casts shows almost a 20% improvement in performance, and during the tests there seems to be less locking contention. ---------------------------------------------------------------- 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: [email protected]
