Skip to content

Conversation

sgup432
Copy link
Contributor

@sgup432 sgup432 commented Oct 3, 2025

Description

I came across this logic in add(), subtract() in NumericUtils where we have branching logic to extract carry/borrow.
Seems like we can better replace it with bitwise logic, and it would be faster.

Copy link
Contributor

github-actions bot commented Oct 3, 2025

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

@kaivalnp
Copy link
Contributor

kaivalnp commented Oct 8, 2025

I tried benchmarking your proposed changes, and looks like subtract is slowed down, while add is marginally faster:

Benchmark                          (size)   Mode  Cnt   Score   Error   Units
NumericUtilsBenchmark.add              16  thrpt   15  68.679 ± 2.212  ops/us
NumericUtilsBenchmark.add              32  thrpt   15  37.675 ± 0.392  ops/us
NumericUtilsBenchmark.add              64  thrpt   15  19.586 ± 0.233  ops/us
NumericUtilsBenchmark.addNew           16  thrpt   15  70.461 ± 0.161  ops/us
NumericUtilsBenchmark.addNew           32  thrpt   15  38.566 ± 0.035  ops/us
NumericUtilsBenchmark.addNew           64  thrpt   15  20.022 ± 0.019  ops/us
NumericUtilsBenchmark.subtract         16  thrpt   15  68.377 ± 2.006  ops/us
NumericUtilsBenchmark.subtract         32  thrpt   15  35.782 ± 0.995  ops/us
NumericUtilsBenchmark.subtract         64  thrpt   15  19.097 ± 0.393  ops/us
NumericUtilsBenchmark.subtractNew      16  thrpt   15  51.872 ± 0.097  ops/us
NumericUtilsBenchmark.subtractNew      32  thrpt   15  27.446 ± 0.012  ops/us
NumericUtilsBenchmark.subtractNew      64  thrpt   15  14.163 ± 0.009  ops/us

See #15303 for my benchmark setup, where I have another proposal to speed these up!

Copy link
Contributor

@jainankitk jainankitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess we can close this PR in favor of ? - #15303

@jainankitk jainankitk closed this Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants