-
Notifications
You must be signed in to change notification settings - Fork 737
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
Remove vmSnapshotFilePath from J9SpecialArguments #20532
Remove vmSnapshotFilePath from J9SpecialArguments #20532
Conversation
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.
While this approach does save on an intermediary pointer, it requires an additional iteration through the args
list. In the worst case, depending on the location of the -Xsnapshot=
option and the size of the list, it may result in a full traversal of a potentially large args
list, which could negatively impact startup performance. Although this incurs additional memory usage, the impact on startup time outweighs the benefit of reduced memory usage. @tajila Thoughts?
Removing the additional memory that a pointer takes up wasn't the main thrust behind this; it is more so a code cleanliness patch. I understand that ideally we would not traverse the To get around this, we could also pass |
Actually, we could just change the Lines 2246 to 2250 in dc42c9a
|
Signed-off-by: Nathan Henderson <[email protected]>
dc42c9a
to
a600cf8
Compare
jenkins test sanity.functional amac jdk21 |
The PR builds will test the changes in the disabled state. @ThanHenderson Did you test these changes in the enabled state; do they look good? |
Yes, I have tested it. All good. |
Signed-off-by: Nathan Henderson [email protected]