Rewrite expressions module with Java 24 classfile API #14597

uschindler · 2025-04-30T17:39:01Z

First step in getting rid of ASM dependency:

Java 24 added the classfile API (https://openjdk.org/jeps/484) in non-preview mode. This allows to generate class files with a builder-based API (unfurtunately its is not the usual ASM visitor desaster, it is more a lambda-inside-lambda desaster).

This PR removes the license files and all ASM dependencies from runtime code (and module descriptors). The code was rewritten. The rewrite may not yet be the most elegant lambda-way to make it, but the PR is easier to review with not so many changes and reshuffled code.

We still have ASM core as a dependncy for Gradle's APIJAR generator, but this one can also be removed soon.

dweiss

Pretty cool.

rjernst

Looks great! It seems like ASM translated relatively well (in this case) to the ClassFile API (much of it was just changing imports/methods to call but looks very similar).

…od describeConstable(). For normal types the optional can never be empty

jdconrad

LGTM! I appreciate the fairly direct replacement as the first step since it made it so easy to review.

rmuir

looks great! also, I trust the tests here have good coverage. Thank you for doing this.

…; wrap ParseExceptions using helper method

uschindler · 2025-04-30T20:35:27Z

I did some final cleanup and added changes entry. Will merge soon.

Thanks for the reviews by @rjernst and @jdconrad, very appreciated.

uschindler · 2025-04-30T21:01:11Z

Looks great! It seems like ASM translated relatively well (in this case) to the ClassFile API (much of it was just changing imports/methods to call but looks very similar).

The hard part was more to understand and figure out how it works. After that transforming the Antlr visitor to Classfile API was trivial.

Very hard was that I was unable to figure out how to get a ConstantDesc which is needed for the bootstrapper from a String or Integer. I had to figure out first that all those classes implement ConstantDesc so you can just pass the String instance as parameter where type ConstantDesc is expected:

lucene/lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java

Line 436 in 938be2a

text);

There are still some nice goodies missing in Classfile API:

No replacement for GeneratorAdaptor#loadArgs and a better named function like GeneratorAdaptor#loadThis (as synonmy for aload(0)). This would make the typical "pass all parameters automatically to the invoke super call" in ctors or overridden methods easier to write.
More methods with TypeKind to also generate compare code without knowing the exact tyes.

dweiss · 2025-04-30T21:24:57Z

The hard part was more to understand and figure out how it works.

For the fun of it, I asked chatgpt to do it. Uwe - you're still much, much better. :)

rmuir · 2025-04-30T22:40:13Z

lol the day we have an army of Uwe AIs, I feel like I've already seen this movie

dweiss · 2025-05-01T06:17:32Z

Uncanny resemblance!

…fficial documentation of classfile API recommends (#14597)

uschindler · 2025-05-01T07:21:17Z

I made a small followup commit to make usage more like recommended by Classfile API docs: e2d0685

uschindler · 2025-05-01T10:16:40Z

Finally here is an example Classfile output with this PR for this code:

  public void testNamespacesWithoutDirectMH() throws Exception {
    Map<String, MethodHandle> functions =
        Map.of(
            "foo.bar",
            MethodHandles.constant(double.class, 9),
            "bar.foo",
            MethodHandles.identity(double.class));
    Expression expr = compile("foo.bar() + bar.foo(7)", functions);
    assertEquals(16, expr.evaluate(null), DELTA);
  }

//  (version 24 : 68.0, no super bit)
public final class org.apache.lucene.expressions.js.JavascriptCompiler$CompiledExpression extends org.apache.lucene.expressions.Expression {
  
  // Method descriptor #6 (Ljava/lang/String;[Ljava/lang/String;)V
  // Stack: 3, Locals: 3
  public JavascriptCompiler$CompiledExpression(java.lang.String arg0, java.lang.String[] arg1);
    0  aload_0 [this]
    1  aload_1 [arg0]
    2  aload_2 [arg1]
    3  invokespecial org.apache.lucene.expressions.Expression(java.lang.String, java.lang.String[]) [8]
    6  return

  
  // Method descriptor #10 ([Lorg/apache/lucene/search/DoubleValues;)D
  // Stack: 5, Locals: 2
  public double evaluate(org.apache.lucene.search.DoubleValues[] arg0) throws java.io.IOException;
     0  ldc <Dynamic> 0 func0 java.lang.invoke.MethodHandle [23]
     2  invokevirtual java.lang.invoke.MethodHandle.invokeExact() : double [29]
     5  ldc <Dynamic> 1 func1 java.lang.invoke.MethodHandle [34]
     7  ldc2_w <Double 7.0> [35]
    10  invokevirtual java.lang.invoke.MethodHandle.invokeExact(double) : double [39]
    13  dadd
    14  dreturn
    15  aload_0 [this]
    16  invokestatic org.apache.lucene.expressions.js.JavascriptCompiler.patchStackTrace(java.lang.Throwable, org.apache.lucene.expressions.Expression) : java.lang.Throwable [45]
    19  athrow
      Exception Table:
        [pc: 0, pc: 15] -> 15 when : java.lang.Throwable
      Stack map table: number of frames 1
        [pc: 15, same_locals_1_stack_item, stack: {java.lang.Throwable}]

Bootstrap methods:
  0 : # 19 invokestatic org/apache/lucene/expressions/js/JavascriptCompiler.dynamicConstantBootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/invoke/MethodHandle;
	Method arguments:
		#12 foo.bar,
  1 : # 19 invokestatic org/apache/lucene/expressions/js/JavascriptCompiler.dynamicConstantBootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/invoke/MethodHandle;
	Method arguments:
		#31 bar.foo
}

The previous version as generated by the old code in branch_10x is:

//  (version 21 : 65.0, super bit)
public final class org.apache.lucene.expressions.js.JavascriptCompiler$CompiledExpression extends org.apache.lucene.expressions.Expression {
  
  // Method descriptor #6 (Ljava/lang/String;[Ljava/lang/String;)V
  // Stack: 3, Locals: 3
  public JavascriptCompiler$CompiledExpression(java.lang.String arg0, java.lang.String[] arg1);
    0  aload_0 [this]
    1  aload_1 [arg0]
    2  aload_2 [arg1]
    3  invokespecial org.apache.lucene.expressions.Expression(java.lang.String, java.lang.String[]) [8]
    6  return

  
  // Method descriptor #10 ([Lorg/apache/lucene/search/DoubleValues;)D
  // Stack: 5, Locals: 2
  public double evaluate(org.apache.lucene.search.DoubleValues[] arg0) throws java.io.IOException;
     0  ldc <Dynamic> 0 func0 java.lang.invoke.MethodHandle [27]
     2  invokevirtual java.lang.invoke.MethodHandle.invokeExact() : double [33]
     5  ldc <Dynamic> 1 func1 java.lang.invoke.MethodHandle [38]
     7  ldc2_w <Double 7.0> [39]
    10  invokevirtual java.lang.invoke.MethodHandle.invokeExact(double) : double [43]
    13  dadd
    14  dreturn
    15  aload_0 [this]
    16  invokestatic org.apache.lucene.expressions.js.JavascriptCompiler.patchStackTrace(java.lang.Throwable, org.apache.lucene.expressions.Expression) : java.lang.Throwable [47]
    19  athrow
      Exception Table:
        [pc: 0, pc: 14] -> 15 when : java.lang.Throwable
      Stack map table: number of frames 1
        [pc: 15, same_locals_1_stack_item, stack: {java.lang.Throwable}]

Bootstrap methods:
  0 : # 23 invokestatic org/apache/lucene/expressions/js/JavascriptCompiler.dynamicConstantBootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/invoke/MethodHandle;
	Method arguments:
		#16 foo.bar,
  1 : # 23 invokestatic org/apache/lucene/expressions/js/JavascriptCompiler.dynamicConstantBootstrap:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/invoke/MethodHandle;
	Method arguments:
		#35 bar.foo
}

This is the same code, all operators are identical. Only differences in constant pool. The changes here are only the class flags no longer having obsolete ACC_SUPER and the Classfile API automatically generates class files for the current Java version (no need to configure anything).

Theres a slight difference in the try/catch. The old version was a bit incorrect, because the return statement was not part of the try block. But this does not matter as a return statement can't throw exceptions.

uschindler · 2025-05-01T17:48:32Z

I made some followup changes here: #14602

Rewrite expressions module with Java 24 classfile API

ea7e1b8

uschindler added the module:expressions label Apr 30, 2025

uschindler added this to the 11.0.0 milestone Apr 30, 2025

uschindler requested a review from rmuir April 30, 2025 17:39

uschindler self-assigned this Apr 30, 2025

github-project-automation bot added this to OpenSearch Lucene & Core Performance Tracking Apr 30, 2025

github-project-automation bot moved this to Open in OpenSearch Lucene & Core Performance Tracking Apr 30, 2025

Remove ACC_SUPER from class modifier (that was ASM internal)

6238b64

dweiss approved these changes Apr 30, 2025

View reviewed changes

uschindler added 2 commits April 30, 2025 19:54

cleanup constants for descriptors

6130281

rename lambda parameter to be consistent with others

d69f024

rjernst approved these changes Apr 30, 2025

View reviewed changes

Rewrite ClassDesc constants by using the correct java.lang.Class meth…

c1ab42a

…od describeConstable(). For normal types the optional can never be empty

jdconrad approved these changes Apr 30, 2025

View reviewed changes

rmuir approved these changes Apr 30, 2025

View reviewed changes

uschindler added 2 commits April 30, 2025 22:05

Rename constants to be in line with Classfile library's ConstantDescs…

82bd145

…; wrap ParseExceptions using helper method

Add CHANGES.txt

938be2a

uschindler merged commit 1b5c25f into apache:main Apr 30, 2025
7 checks passed

github-project-automation bot moved this from Open to Merged in OpenSearch Lucene & Core Performance Tracking Apr 30, 2025

uschindler deleted the dev/rewriteExpressionClassfileAPI branch April 30, 2025 21:06

dweiss mentioned this pull request May 1, 2025

Apache jenkins nodes use java 24-ea #14600

Closed

asfgit pushed a commit that referenced this pull request May 1, 2025

Use types directly using describeConstable().orElseThrow() like the o…

e2d0685

…fficial documentation of classfile API recommends (#14597)

Rewrite expressions module with Java 24 classfile API #14597

Rewrite expressions module with Java 24 classfile API #14597

Uh oh!

Conversation

uschindler commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dweiss left a comment

Choose a reason for hiding this comment

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

jdconrad left a comment

Choose a reason for hiding this comment

Uh oh!

rmuir left a comment

Choose a reason for hiding this comment

Uh oh!

uschindler commented Apr 30, 2025

Uh oh!

uschindler commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dweiss commented Apr 30, 2025

Uh oh!

rmuir commented Apr 30, 2025

Uh oh!

dweiss commented May 1, 2025

Uh oh!

uschindler commented May 1, 2025

Uh oh!

uschindler commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uschindler commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

uschindler commented Apr 30, 2025 •

edited

Loading

uschindler commented Apr 30, 2025 •

edited

Loading

uschindler commented May 1, 2025 •

edited

Loading