RFR: 8275338: Add JFR events for notable serialization situations [v12]

[Raffaello Giulietti]

On Mon, 8 Jan 2024 18:15:36 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8275338
>>  - Simplified event messages.
>>    Remove ckecker allocation.
>>  - Corrected @Label of event and of field.
>>  - Removed @module from test.
>>  - Merge branch 'master' into 8275338
>>  - Renamed an event field.
>>  - Minor changes.
>>  - Removed event kind.
>>    serialVersionUID must have type long.
>>    Test now base on keyword search in event message.
>>    Commented test classes about misdeclarations.
>>  - Changes according to reviewer's comments.
>>  - Better name for a label, corrected name of removed field.
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/966dac39...9ca1f36d
>
> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 53:
> 
>> 51:     private static final Class<?>[] READ_OBJECT_NO_DATA_PARAM_TYPES = {};
>> 52:     private static final Class<?>[] WRITE_REPLACE_PARAM_TYPES = {};
>> 53:     private static final Class<?>[] READ_RESOLVE_PARAM_TYPES = {};
> 
> These could share a single zero length Class<?> array.

Conceptually, these are independent types. There's no logical relationship between them. Sharing a zero length array would convey a false sense of logical sharing.

> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 87:
> 
>> 85:         }
>> 86:         if (cl.isEnum()) {
>> 87:             commitEvent(cl, SUID_NAME + " in an enum class is not effective");
> 
> Is there a best practice that can be included in the message? "SUID should not be declared"?

Yes, that's perhaps clearer, will do.

> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java line 113:
> 
>> 111:         } else if (cl.isEnum()) {
>> 112:             commitEvent(cl, SERIAL_PERSISTENT_FIELDS_NAME +
>> 113:                     " in an enum class is not effective");
> 
> Is there best practice to include in the message? "SPFN should not be declared"?

Yes, that's perhaps clearer, will do.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445922807
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445924385
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445924424