Bug #10071: Method code too large - ReflectASM - Runtime Infrastructure - Golden Code Redmine

Bug #10071

Method code too large - ReflectASM

Added by Teodor Gorghe about 1 year ago. Updated 6 months ago.

Status:

Closed

Priority:

Normal

Assignee:

Teodor Gorghe

Target version:

Start date:

Due date:

% Done:

100%

billable:

vendor_id:

GCD

case_num:

version_reported:

version_resolved:

reviewer:

Constantin Asofiei

production:

env_name:

topics:

Description

When the bytecode is too large, we get Method code too large!.
To address this, we need to:
- Investigate if we can replace ReflectASM with standard Java Reflection Method.invoke.
- Benchmark the performance impact of this change, particularly focusing on potential improvements with OpenJDK 17.

create.sh (1.11 KB) Teodor Gorghe, 05/30/2025 08:26 AM

p100.p (834 KB) Teodor Gorghe, 05/30/2025 08:26 AM

Related issues

History

#1 Updated by Teodor Gorghe about 1 year ago

#3 Updated by Teodor Gorghe about 1 year ago

Status changed from New to WIP
Assignee set to Teodor Gorghe

Created branch 10071a. I am currently searching for a simple test case.

#4 Updated by Constantin Asofiei about 1 year ago

Teodor Gorghe wrote:

Created branch 10071a. I am currently searching for a simple test case.

Create a .p with 100s procedure/functions and each with 10s of args. After that use RUN to execute it.

#5 Updated by Teodor Gorghe about 1 year ago

File p100.p added
File create.sh added

Constantin Asofiei wrote:

Create a .p with 100s procedure/functions and each with 10s of args. After that use RUN to execute it.

I have managed to get it on Method code too large! with 500 functions and procedures, with 20 args each.

25/05/30 15:23:11.017+0300 |  SEVERE | com.goldencode.p2j.main.StandardServer [StandardServer.invoke()] | ThreadName:Conversation [00000001:bogus], Session:00000001, ThreadId:00000005, User:bogus | Abnormal end!
java.lang.RuntimeException: invoke() of class com.goldencode.testcases.PBytecodeCall and  method execute failed
    at com.goldencode.p2j.util.ControlFlowOps.invokeError(ControlFlowOps.java:8535)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6718)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6487)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1371)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:958)
    at com.goldencode.p2j.main.StandardServer$LegacyInvoker.execute(StandardServer.java:2870)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2202)
    at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:704)
    at com.goldencode.p2j.main.StandardServerMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:418)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:237)
    at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.RuntimeException: invoke() of class com.goldencode.testcases.PBytecode and  method execute failed
    at com.goldencode.p2j.util.ControlFlowOps.invokeError(ControlFlowOps.java:8535)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7685)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4572)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7189)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7092)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1447)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1429)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1377)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:427)
    at com.goldencode.testcases.PBytecodeCall.lambda$execute$0(PBytecodeCall.java:25)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9613)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:9221)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:691)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:664)
    at com.goldencode.testcases.PBytecodeCall.execute(PBytecodeCall.java:23)
    at com.goldencode.testcases.PBytecodeCallMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9761)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9717)
    at com.goldencode.p2j.util.ControlFlowOps.invokeExternalProcedure(ControlFlowOps.java:6629)
    ... 12 more
Caused by: java.lang.RuntimeException: Method code too large!
    at com.esotericsoftware.asm.MethodWriter.a(Unknown Source)
    at com.esotericsoftware.asm.ClassWriter.toByteArray(Unknown Source)
    at com.esotericsoftware.reflectasm.MethodAccess.get(MethodAccess.java:275)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller$CacheValue.<init>(ControlFlowOps.java:11081)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.putCache(ControlFlowOps.java:10905)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.valid(ControlFlowOps.java:10694)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.valid(ControlFlowOps.java:9857)
    at com.goldencode.p2j.util.ControlFlowOps.validArgumentsInt(ControlFlowOps.java:8174)
    at com.goldencode.p2j.util.ControlFlowOps.validArguments(ControlFlowOps.java:8124)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7539)
    ... 30 more

#6 Updated by Constantin Asofiei about 1 year ago

If the stack trace matches the one in #10068, then first step is to bypass this (so Method.invoke is used if ReflectASM fails) and re-run all unit-tests from #10068, and check for other failures (will discuss at that task).

Second step is to remove it completely (in a branch) ReflectASM and do performance testing.

#7 Updated by Teodor Gorghe about 1 year ago

Committed revision 15952 on task branch 10071a.
This change includes only the replacement of ReflectASM with Java Reflection API on ControlFlowOps.java, MethodInvoker.java, Utils.java.
ReflectASM is still being used on WidgetCondigDef.java, ClientConfigManager.java and ConfigManager.java, where there is actually a performance boost compared to Java Reflection.

#8 Updated by Teodor Gorghe about 1 year ago

% Done changed from 0 to 50

#9 Updated by Teodor Gorghe about 1 year ago

Status changed from WIP to Review

#10 Updated by Constantin Asofiei about 1 year ago

Does Utils.invoke need to make method.setAccessible(true)? The test is declaring a PROCEDURE proc0 PRIVATE. and using a RUN proc0.

Otherwise, the only left dependency is in WidgetConfigDef, which uses FieldAccess. Lets leave this in place for now.

#11 Updated by Teodor Gorghe about 1 year ago

Constantin Asofiei wrote:

Does Utils.invoke need to make method.setAccessible(true)? The test is declaring a PROCEDURE proc0 PRIVATE. and using a RUN proc0.

Otherwise, the only left dependency is in WidgetConfigDef, which uses FieldAccess. Lets leave this in place for now.

The case you provided uses ControlFlowOps.invoke, which goes into ControlFlowOps.invokeImpl. This method sets mthd.setAccessible(true), so the test passes. I have made a simple progress program and I can run this private internal procedure in the same procedure.

Utils.invoke is used mostly by the persistence layer and I am finding a test case where there is a private/protected method which might fail.

#12 Updated by Constantin Asofiei about 1 year ago

I don't think you need to use the catch block, Utils.invoke already throws it?

      try
      {
         return method.invoke(instance, args);
      }
      catch (InvocationTargetException e)
      {
         throw new RuntimeException(e.getCause());
      }

#13 Updated by Teodor Gorghe about 1 year ago

ReflectASM throws RuntimeException with the cause from which the invoked method throws.
Java Reflection Invoke method throws InvocationTargetException (which is specified in method declaration) and I'm investigating right now if there is somewhere where this exception is not treated.

#14 Updated by Teodor Gorghe about 1 year ago

Committed revision 15954 on task branch 10071a.
- replaced RuntimeException with InvocationTargetException (removing try-catch block). It seems that this exception is treated on all usages.

#15 Updated by Constantin Asofiei about 1 year ago

Status changed from Review to Internal Test

Teodor Gorghe wrote:

Committed revision 15954 on task branch 10071a.
- replaced RuntimeException with InvocationTargetException (removing try-catch block). It seems that this exception is treated on all usages.

Is there more testing needed?

#16 Updated by Teodor Gorghe about 1 year ago

Lorian, Stefanel, Andreea told that there are no issues.
I have asked Serban to check if he ran with this branch. I don't have any answers from Danut about his project.

#17 Updated by Constantin Asofiei about 1 year ago

Status changed from Internal Test to Merge Pending
% Done changed from 50 to 100

Please merge after 9447a.

#18 Updated by Teodor Gorghe about 1 year ago

% Done changed from 100 to 50
Status changed from Merge Pending to Test

Branch 10071a was merged to trunk rev 15983 and archived.

#19 Updated by Teodor Gorghe about 1 year ago

% Done changed from 50 to 100

#20 Updated by Teodor Gorghe about 1 year ago

I found another issue, after changing the type of exception from RuntimeException to InvocationTargetException. The issue is that on OEUnit and ABLUnit, there are some tests that fails, but appears as successful.
The following code from Dispatcher.java:

846               // try to extract original exception
847               if (t instanceof InvocationTargetException)
848               {
849                  InvocationTargetException ite = (InvocationTargetException) t;
850                     
851                  Throwable cause = ite.getCause();
852
853                  if (cause instanceof Exception)
854                  {
855                     exception = cause;
856                  }
857               }

When the cause is not an instance of Exception, like ProError, that will keep exception as InvocationTargetException.
Changing the condition from line 853 with cause != null, fixes the "fake" passing tests.

#21 Updated by Teodor Gorghe about 1 year ago

Status changed from Test to WIP

Created task branch 10071b.

#22 Updated by Teodor Gorghe about 1 year ago

Status changed from WIP to Review

Committed revision 15985 on task branch 10071b:
- Regression from 10071a: fixed error handling on processInbound. This fixes the false positives on tests that fail with AssertionFailedError.

#23 Updated by Constantin Asofiei about 1 year ago

Status changed from Review to Merge Pending

Go ahead and merge now.

#24 Updated by Teodor Gorghe about 1 year ago

Branch 10071b was merged to trunk rev 15988 and archived.

#25 Updated by Teodor Gorghe about 1 year ago

Status changed from Merge Pending to Test

#26 Updated by Teodor Gorghe 7 months ago

Related to Feature #11004: Optimizing Java Reflection Method.invoke added

#27 Updated by Constantin Asofiei 6 months ago

Status changed from Test to Closed

Also available in: Atom PDF

Project

General

Profile

FWD » Core Development » Runtime Infrastructure

Issues

Custom queries