jainankitk commented on PR #14516: URL: https://github.com/apache/lucene/pull/14516#issuecomment-2817678029
> But the benchmark is not useful, as it was written as a test. Tests don't run normally, in particular without java C2 compiler, so trying to make benchmark via test is not useful and biased in many ways. > > If you want to make a microbenchmark, please use `lucene/benchmark-jmh` facility. Thanks @rmuir for the suggestion. I have added couple of jmh benchmarks for this, and they clearly show, probably not the right change. While the slowdown for `fromPointDistanceStandardSin` / `fromPointDistanceSloppySin` is about ` 0.78x` compared to `.49x` in `sloppySin` vs `standardSin`, and it is probable we don't even observe this micro-level slowdown in practical applications, there is no good reason for merging this change as is. I have just moved `sloppySin` from `GeoUtils` to `SloppyMath` for keeping all the sloppy stuff in single place. Also added clarifying comment about possible regression pointing to the benchmarks. Please let me know if you have any concerns with that. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
