-
Notifications
You must be signed in to change notification settings - Fork 21
CNDB-15990: Avoid varargs allocations in atomicMoveWithFallback #2124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Checklist before you submit for review
|
eolivelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch is missing unit tests
are there already unit tests that covers the methods you are changing ?
| try | ||
| { | ||
| Files.copy(from, to, option); | ||
| Files.copy(from, to, new CopyOption[] { option }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still allocate a new array here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes there already are unit tests that cover the methods I've changed. https://github.com/datastax/cassandra/blob/main/test/unit/org/apache/cassandra/io/util/PathUtilsTest.java
Fixed the unnecessary alocation.
eolivelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
✔️ Build ds-cassandra-pr-gate/PR-2124 approved by ButlerApproved by Butler |



What is the issue
https://github.com/riptano/cndb/issues/15990
What does this PR fix and why was it fixed
What: Eliminates unnecessary array allocations in PathUtils.atomicMoveWithFallback() by pre-allocating CopyOption[] arrays as static constants.
Why: JFR profiling showed this method was a memory allocation hotspot during RemoteFileCache preloading. Each call to Files.move() with varargs parameters created new arrays, causing excessive GC pressure.