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
WFCORE-6369 Eliminate unnecessary collection copying in AbstractAddStepHandler constructors #5535
Conversation
controller/src/main/java/org/jboss/as/controller/AbstractAddStepHandler.java
Outdated
Show resolved
Hide resolved
…epHandler constructor
Collect into list rather than set for heap efficiency, and because AttributeDefinition does not implement hashCode/equals.
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, @bstansberry you already did a review, anything else from your side?
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.
This approach changes the semantics of the protected 'attributes' field by making it immutable, but that's fine with me; as I'd have been ok with making it private.
We should follow this up by deprecating the field an adding a 'protected final Collection getAttributes()' javadoced as returning an immutable collection. But that doesn't need to block merging this.
I created https://issues.redhat.com/browse/WFCORE-6419 to follow up on the last comment |
Thanks @pferraro / @bstansberry |
https://issues.redhat.com/browse/WFCORE-6369
Fix improper use of Set in AbstractAddStepHandler.Parameters, since AttributeDefinition is not hashable.