RFR: 8112: Flamegraph model creation performance improvements [v4]
Brice Dutheil
bdutheil at openjdk.org
Tue Sep 5 18:33:53 UTC 2023
On Tue, 5 Sep 2023 17:59:07 GMT, Vincent Alexander Beelte <duke at openjdk.org> wrote:
>> core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/stacktrace/tree/Node.java line 97:
>>
>>> 95: public Integer getNodeId() {
>>> 96: return nodeId;
>>> 97: }
>>
>> **issue:** This is a breaking change. And should be reverted.
>
> Oh, did someone add a call to this? That's unfortunate. Overall this was not that important for performance. It was however also used in the equals method, which I think was wrong. The nodeId is computed using hashing so in theory there might be collisions. Using that for equals would mean Nodes are only compared to be "kind of" equal and not exactly equal.
> I did however also never find a reference where the equals would have been called. I even had a breakpoint in it that never was hit.
It's probable. This could be used in tests in other libraries, or else. The core libraries are distributed on maven, so whether it is used or not such API changes should be avoided.
That said I understand the change to make this lazy. I think this is alright to drop the node id from the equals.
-------------
PR Review Comment: https://git.openjdk.org/jmc/pull/502#discussion_r1316254248
More information about the jmc-dev
mailing list