On Sat, 16 Dec 2023 01:27:17 GMT, Erik Gahlin <egah...@openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Restrict accessibility of nested classes. > > It seems correct to have the event disabled in both configurations. It's not > hard to imagine pathological cases where millions of stream objects are > created, which would create overhead and flood event buffers, making JFR not > usable. > > That said, I wonder how many will actually use, or find out about the event, > if they need to edit the .jfc file or specify > -XX:StartFlightRecording:jdk.SerializationMisdeclaration#enabled=true on > command line? > > How important is the stack trace when diagnosing serialization > misdeclarations? > > We faced a similar situation with the Finalization event a couple of years > ago, but we were able to make the event periodic and enabled by default with > some VM magic. An event that is enabled by default will probably be seen by > 10 000 times as many users, so it might be worth considering other > approaches, even if they are harder to implement, or we lose some of the > information. > @egahlin Yes, disabled by default means less overhead in the usual case. > > On the other hand, the checks and the events generation are done for > serializable classes that actually actively participate in serialization, > lazily, and just once per class, so maybe they are not as invasive and might > be "always enabled". Pathological cases with millions of serializable classes > might indeed lead to problems with "always enable", although they should be > quite rare. > If it's once per class per JVM, I think it's fine to enable it in profile.jfc. It's meant to have low overhead (around 1%) for the typical application, but OK with higher in pathological cases. Loading/initiating a class has a cost, so it will likely dominate. > I don't think that stack traces are that useful for this kind of event, since > they are mostly related to the static class structure rather than to runtime > characteristics. Looking at the event itself should help in locating the > problem quickly: it reports the class, the member, and what's questionable > with it. > If we were to make the event periodic, it's hard to keep the stack trace around, at least in Java, so I wanted to know how important it is. Now if we can have it in profile.jfc, I think it's fine to keep it. If you think the stack trace has no use at all, then you could remove it and perhaps also the thread using the RemoveFields annotation. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17129#issuecomment-1860765344
