nqp icon indicating copy to clipboard operation
nqp copied to clipboard

[JVM] Necromance a cheaper (de)serialization

Open Kaiepi opened this issue 2 years ago • 3 comments

This started as means of fixing https://github.com/rakudo/rakudo/issues/4952 for the next stretch of https://github.com/Raku/nqp/pull/776, but is kind of its own rabbit hole. The hs_err log produced by the failure in CORE.d.setting points to this line as the source of the error from within nqp:

https://github.com/Raku/nqp/blob/4f4d4ef1f6776a0a169441f18060ad6863ff5110/src/vm/jvm/runtime/org/raku/nqp/runtime/LibraryLoader.java#L71

While looking around here, I noticed we are very lackadaisical with how we handle serialized bytecode within memory. Namely, when LibraryLoad.load is invoked, we can wind up copying entire files in memory multiple times over, and their decompressed classfile and bytecode files persist in memory as long as their classes do. I haven't quite managed to even provoke a change in the CORE.d.setting error produced, but I have managed to convince CORE.c.setting to build in 90% of the time it does on master by reducing the copying to just one instance so far. This can be taken further to eliminate copying of files in bulk entirely, but will take time.

Passes make test with OpenJDK 19-ea, I guess.

Kaiepi avatar Aug 04 '22 06:08 Kaiepi

Turns out -XX:+UseTransparentHugePages is a detriment! CORE.c.setting can now build 23% faster than on master.

Kaiepi avatar Aug 04 '22 07:08 Kaiepi

There's more that can be done in this vein in the SerializationReader whereabouts, but I'd rather not mess with both the library loader and serialization in general at the same time. CI seems to enjoy these changes as-is.

Kaiepi avatar Aug 09 '22 15:08 Kaiepi

This looks/sounds very promising.

I don't feel qualified to give it a real review, but I'll at least try to test the branch. (But that will take some time.)

usev6 avatar Sep 19 '22 18:09 usev6

Given this unblocks Kaiepi, what hinders merging this as is? It doesn't seem to touch MoarVM things, so can't break thinks too badly.

patrickbkr avatar Oct 21 '22 15:10 patrickbkr

A spectest looked good as well.

usev6 avatar Oct 23 '22 06:10 usev6

Given this unblocks Kaiepi, what hinders merging this as is? It doesn't seem to touch MoarVM things, so can't break thinks too badly.

From me it's a +1 for merging.

If no-one else chimes in I'll hit the merge button in a couple of days.

usev6 avatar Oct 24 '22 19:10 usev6