richardstartin commented on code in PR #2072:
URL: https://github.com/apache/helix/pull/2072#discussion_r860859346
##########
helix-core/src/main/java/org/apache/helix/HelixProperty.java:
##########
@@ -39,7 +39,7 @@
* A wrapper class for ZNRecord. Used as a base class for IdealState,
CurrentState, etc.
*/
public class HelixProperty {
Review Comment:
Of course, this does not clone the `ZNRecord`, so code which assumes the
`ZNRecord` has been copied will not work properly, but these constructors are
made available for advanced users of Helix, like Apache Pinot. In our code, the
copy is needless and shows up as a surprisingly noticeable cost, which I would
like to avoid incurring. This is why I took care to document the constructors;
use at your own risk, take responsibility for it, but make it possible not to
incur the cost.
Subclassing would not work because `HelixProperty` would need to be
constructed to construct the subclass, which would result in a clone. It could
work the other way around though, but this flag feels like a simple, easy to
document, and backward compatible solution.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
