On 6/17/20 6:50 PM, Mandy Chung wrote:
> On 6/17/20 8:11 AM, Aleksey Shipilev wrote:
>> On 6/15/20 11:58 PM, Mandy Chung wrote:
>> Amusingly, with the rest of Unsafe available the barn is still full open.
>> Here's the sketch for the
>> workaround: get the field list with getDeclaredFields, optionally guess the
>> VM layout (the rules are
>> not that hard, and JOL does it routinely), instantiate a Record with
>> whatever arguments, poke the
>> test patterns with Unsafe.put*, read them back from record components to
>> verify. It would get me the
>> same objectFieldOffset in a round-about way.
>
> I'm aware of these workarounds in the wild. As sun.misc.Unsafe is JDK
> unsupported API, this patch
> does not attempt to implement a complete solution but adds an obvious
> notification informing the
> frameworks that `sun.misc.Unsafe::objectFieldOffset` and
> `sun.misc.Unsafe::staticField{Offset/Base}`
> not to support records.
I understand the intent. My point is that intent is mistimed during this time.
I understand this
tripwire needs to be put while Records are still in preview. My point is that
it cannot be put in
before Records serialization story has the preview-proven answer.
The intent also looks rather opaque. The intent is to notify maintainers, fine.
I am one of the
maintainers (although not of serialization library, but still heavy-Unsafe
one), so hear me out. I
came up with the workaround above in about 15 minutes after someone pointed out
the exception to me.
Have it crossed my mind that maybe JDK developers want some help here? No. It
looks like another
impediment that should be solved on the spot. Does that exception communicate
any intent at all? The
message is generic. There is no comment. How would you expect to push
maintainers to think along the
"we should be collaborating to find a proper way to do this", instead of "this
is set in stone; let
me hack this around too"?
This is the actionable bit:
At very least the exception message should say something about the intent. And
maybe even the
comment in Unsafe.java should point to the discussion about this intent and
maybe even provide the
breadcrumbs to follow, e.g. to ObjectInputStream.readRecord().
>> Are we absolutely sure that what ObjectInputStream.readRecord() does fits
>> the 3rd party
>> serialization libraries? Does it work for them performance-wise?
>
> There should be performance improvement opportunity (see Peter Levart's good
> work - JDK-8247532 [1])
>
>> (Are we even sure about that for JDK itself?)
>
> Java serialization support for records use its canonical constructor. (I'm
> not sure what you tried
> to point out by this).
Let me try again.
The existence of JDK-8247532 solidifies my concern: the JDK code itself had not
yet figured out how
to do Record serialization fast! Now put yourself in the shoes of the 3rd party
lib maintainer:
Unsafe is forbidden, JDK code itself is slow. Asking 3rd party lib maintainer
to find whatever
intricate incantation they should use to get decent performance -- while JDK
itself is still working
on it! -- puts them into weird position, from where the *sane* way out is to
hack around the Unsafe
restriction. I mean, it is as enticing as hacking the JDK code itself, with the
added benefit of
working across the JDK releases.
This is time and again the core of Unsafe discussion, which boils down to a
very simple request: if
JDK developers take away some Unsafe APIs, 3rd party developers need to have
the known good
replacement for it. If that does not happen, 3rd party developers would invest
in doing more Unsafe
hacks.
Record serialization seems to fall into the same trap. To reiterate: the
non-Unsafe code should be
proven as the viable alternative before breaking Unsafe. Otherwise, we only
deepen the dependency on
Unsafe.
>> Because if it is not, 3rd party lib maintainers would proceed to hacking in
>> the
>> "objectFieldOffset workaround" and we would get the cobra-effect-like
>> strengthening of dependency on
>> Unsafe quirks.
>
> This is exactly why we request this for 15 so that 3rd-party lib maintainers
> can prototype and send
> feedback. We love to understand any stumbling block and work together to
> resolve any issue before
> records exit preview.
Again, JDK-8247532 is the writing on the wall: we don't need 3rd party
developers to tell if Record
serialization works fast in 15 -- we already know it does not.
>> It is fun to consider JNI to be more powerful than Unsafe. This seems
>> backwards. The intent to break
>> Unsafe users might be defensible, but this power oddity is still quite odd.
>
> I think you misread the message. There is no claim whether JNI or Unsafe is
> more powerful.
I don't think I did. Disallowing Unsafe to do something that JNI is allowed to
do is making Unsafe
less powerful. This was nothing to do with what API is supported or not.
--
Thanks,
-Aleksey
