RFR: 8279619: [JVMCI] improve EncodedSpeculationReason

[Tom Rodriguez]

On Mon, 20 Feb 2023 19:14:15 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Then I would simply document that only the id is emitted and the creator of the encoding is responsible for any extra disambiguation if the id isn't sufficient.  Or we could skip this PR completely since our strategy for handing out ids protects us from this problem.
>> 
>> I've never been a fan of the hash strategy used by these encodings.  I have no sense of how likely collisions are and how well SHA-1 performs when hashing the very short chunks of data that we're actually feeding them.  I think most of the time the actual data is shorter than the size of a SHA-1 digest.  The most common speculations are id,method,bci which should only be 16 bytes.  If we ever do a redesign of speculations I think it would be useful if they could be printed by HotSpot so that LogCompilation could report them.  Being able to decode failed speculations from the Graal side would be useful as well.
>
>>  I think most of the time the actual data is shorter than the size of a SHA-1 digest
> 
> In that case we just use the data [without hashing it](https://github.com/openjdk/jdk/blob/0bf3a53e01818aca5e365ee7275e567aad0273cc/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotSpeculationEncoding.java#L205).
> 
>> our strategy for handing out ids protects us from this problem
> 
> Well it does now after we fixed a bug where the ids being handed out were not unique. In fact, that bug is why this issue exists. Surely it doesn't hurt to make this API more robust?

It's fine to include it but it's an ad hoc workaround for a problem we no longer have.  We really should be verifying that 2 EncodedSpeculationReasons that are not equal also have different encodings.  That would have likely have caught the original problem but I think it also precludes the use of a hash for larger encodings.  It seems like in practice we probably never use the hash strategy, which is good from my perspective.  I would prefer a hard limit on the encoding size instead.

-------------

PR: https://git.openjdk.org/jdk/pull/12532