LambdaMetafactory requires full privilege access, but doesn't seem to actually restrict functionality

[Steven Schlansker]

> On Jan 18, 2022, at 6:43 AM, Michael Kuhlmann <jdk at fiolino.de> wrote:
> 
> Hi Steven!
> 
> Sorry for coming back so lately; the main reason is that I can't answer your question. The issue you are describing indeed is a bit weird, and I can only speculate that it's like this because LambdaMetaFactory was mainly designed to be used in bootstrap methods for invokedynamic calls. So the checks are probably more restrictive than it should be when being used in standard Java code.

Thanks. I had a suspicion that this would be the answer :)

> But I'm curious about your motivation on using Lambdas at all. Of course, I don't have detailed insights and may miss some important context. Maybe my idea is completely wrong and nonsense; in that case I apologize for being so intrusive. But I was already wondering in the past why Jackson is using bytecode generation when simple MethodHandle combinations would be much easier to use.
> 
> Lambdas like Function and BiConsumer seem to be a bad choice for me. There are two main downsides to it:
> - LambdaMetaFactory only accepts DirectMethodHandles. At least it allows simple type conversions as autoboxing and hierarchical castings, but that's it. Even a simple conversion from a String input (what you probably have when parsing a JSON structure) to a numeric value is impossible to integrate.

Right, the optimization I have written is not so complete as to build the full conversion stack as MethodHandles.

Essentially we are using it only to replace Method[Handle].invoke(), since at the time it was written both carried somewhat of a performance penalty:
https://www.optaplanner.org/blog/2018/01/09/JavaReflectionButMuchFaster.html

Jackson's core implementation reads a stream of tokens, and given a Bean-like object, will react to properties by parsing values and calling a setter.
The tokenizer already takes care of all conversions: there's a hand-rolled UTF8 parser that will emit primitives or String values from the token stream.

This optimization only covers the "calling a setter" or "calling a getter" part of the process in response to tokens from the parser.
We do have specializations for common primitives (int, long, double) to avoid boxing.

So Jackson core will read a property name, deserialize e.g. a long or String or nested object, and then need to call a method on the object to store the (possibly primitive) value.

> - I assume you plan to iterate over the generated lambdas. In this case, Hotspot won't be able to optimize much because each Function/BiConsumer has a different implementation. It's impossible to predict much or even inline much. I would expect the performance to be much lower than the current one.

Interestingly, that wasn't what I found.  At least as of Java 14, the Metafactory generation approach (as limited as my first pass was) was competitive with CGLib hand-generated code in many (but not all) cases, and significantly faster than Method.invoke:
https://github.com/FasterXML/jackson-modules-base/tree/2.14/blackbird/benchmarks
Unfortunately I don't have a MethodHandle.invokeExact comparison in that benchmark. I should go back and finish that work, and re-run it all on 17. If we can call non-static MethodHandles with little overhead, then any reason to use the Metafactory here goes away.

I did try to explore how the inlining worked out using jitwatch but I ran into problems: https://github.com/AdoptOpenJDK/jitwatch/issues/282  

> 
> Why not just concatenating MethodHandles directly? The strategy could be as the following:
> - Use filterArguments to make the getter's return type and the setter's argument convertable
> - Use filterArguments again to directly inject the getter into the setter's argument; so getter and setter will merge into one "transfer" handle
> - Use foldArguments to consecutively perform the transfer handle for each attribute; the resulting handle will transfer all attributes from source to destination
> - Use foldArgument again with MethodHandles::identity to let this transfer handle return its argument
> - And again, use foldArguments with the constructor handle to convert the resulting handle into a simple one-argument-handle expecting the source as a parameter and the newly creation target element as the return value.
> - Use such a handle also for type conversion of nested Pojos (for to-many relations, you have to be creative).
> 
> As a result, you will have only one simple MethodHandle to convert the whole structure at once.
> As a background, back in times I was writing a library to convert Solr result objects to standard Pojos. The main logic was to convert Maps to nested Pojos and vice versa. I used exactly that strategy. It as fun, much better than solving Sudoku ;), and it worked well even with Java 8 features. With the new loop and iterate handles introduced with Java 9, you have even more options.

Cool! Yes, all the new MethodHandle factories are quite interesting.

> Really, I'm curious if this could be an approach for Jackson. Or if not, what could be the obstacles.

I think the problem here is just slightly different: your approach will copy from one Java object to another Java object, but what we are doing is reacting to a token stream and trying to bind it to Java objects with as little overhead as possible.

> 
> 
> On 1/12/22 9:56 PM, Steven Schlansker wrote:
>> Hi core-libs-dev,
>> I am maintaining a module for the popular Jackson JSON library that attempts to simplify code-generation code without losing performance.
>> Long ago, it was a huge win to code-generate custom getter / setter / field accessors rather than use core reflection. Now, the gap is closing a lot with MethodHandles, but there still seems to be some benefit.
>> The previous approach used for code generation relied on the CGLib + ASM libraries, which as I am sure you know leads to horrible-to-maintain code since you essentially write bytecode directly.
>> Feature development basically stopped because writing out long chains of `visitVarInsn(ASTORE, 3)` and the like scares off most contributors, myself included.
>> As an experiment, I started to port the custom class generation logic to use LambdaMetafactory. The idea is to use the factory to generate `Function<Bean, T>` getter and `BiConsumer<Bean, T>` setter implementations.
>> Then, use those during (de)serialization to access or set data.  Eventually hopefully the JVM will inline, removing all (?) reflection overhead.
>> The invocation looks like:
>> var lookup = MethodHandles.privateLookupIn(targetClass, MethodHandles.lookup()); // allow non-public access
>> var getter = lookup.unreflect(someGetterMethod);
>> LambdaMetafactory.metafactory(
>>   lookup,
>>   "apply",
>>   methodType(Function.class),
>>   methodType(Object.class, Object.class),
>>   getter,
>>   getter.type())
>> This works well for classes from the same classloader. However, once you try to generate lambdas with implementations loaded from a different classloader, you run into a check in the AbstractValidatingLambdaMetafactory constructor:
>> if (!caller.hasFullPrivilegeAccess()) {
>>   throw new LambdaConversionException(String.format(
>>     "Invalid caller: %s",
>>     caller.lookupClass().getName()));
>> }
>> The `privateLookupIn` call seems to drop MODULE privilege access when looking across ClassLoaders. This appears to be because the "unnamed module" differs between a ClassLoader and its child.
>> This happens without the use of modulepath at all, only classpath, where I would not expect module restrictions to be in play.
>> Through some experimentation, I discovered that while I cannot call the LambdaMetafactory with this less-privileged lookup, I am still allowed to call defineClass.
>> So, I compile a simple class:
>> package <targetclasspackage>;
>> class AccessCracker { static final Lookup LOOKUP = MethodHandles.lookup(); }
>> and inject it into the target class's existing package:
>> lookup = lookup.defineClass(compiledBytes).getField("LOOKUP").get(null);
>> and now I have a full privileged lookup into the target classloader, and the Metafactory then seems to generate lambdas without complaint.
>> This workaround seems to work well, although it's a bummer to have to generate and inject these dynamic accessor classes.
>> It feels inconsistent that on one hand my Lookup is not powerful enough to generate a simple call-site with the Metafactory,
>> but at the same time it is so powerful that I can load arbitrary bytecode into the target classloader, and thus indirectly do what I wanted to do in the first place (with a fair bit more work)
>> There's a bit of additional context here:
>> https://github.com/FasterXML/jackson-modules-base/issues/138
>> https://github.com/FasterXML/jackson-modules-base/pull/162/files
>> Any chance the Metafactory might become powerful enough to generate call sites even across such unnamed Modules in a future release? Or even more generally across arbitrary Modules, if relevant access checks pass?
>> I'm also curious for any feedback on the overall approach of using the Metafactory, perhaps I am way off in the weeds, and should just trust MethodHandles to perform well if you use invokeExact :) JMH does seem to show some benefit though especially with Graal compiler.
>> Thanks a bunch for any thoughts,
>> Steven Schlansker