search

Raku / nqp

NQP
Other
336 stars 131 forks source link

[JVM] Necromance a cheaper (de)serialization #777

Closed Kaiepi closed 1 year ago

Kaiepi commented 2 years ago

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 commented 2 years ago

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

Kaiepi commented 2 years ago

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.

usev6 commented 1 year ago

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.)

patrickbkr commented 1 year ago

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.

usev6 commented 1 year ago

A spectest looked good as well.

usev6 commented 1 year ago

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.