StackOverflowError after upgrading eclipse compiler

[HarryWeppner]

Background

We have a JUnit test intended to give us an early signal if the stack memory usage changes significantly (enough). This test did its job and started failing after upgrading ECJ from 4.27 to 4.33.

After bisecting further, I found that the test started failing with 4.29. That narrowed down the list of commits to 90 (compared to 4.28).

I used javap -c <class-file> on several class files for Java classes found on the test's stack trace and compiled them with 4.28 and 4.29. The most glaring change difference was the use of

InvokeDynamic #6:makeConcatWithConstants:(Ljava/lang/String;)Ljava/lang/String;

in 4.29 vs. a StringBuilder.append chain in 4.28.

That led me to #1139 and its implementation of JEP 280: Indify String Concatenation. I used that insight to identify a minimal change to get passing and failing test behavior by setting org.eclipse.jdt.core.compiler.codegen.useStringConcatFactory to disabled and enabled, respectively.

Question

@jarthana , @iloveeclipse, since you were primarily involved in #1139, would you expect a different stack memory usage, or is there a lurking bug?

[iloveeclipse]

Interesting question. Could you please share your reproducer, ideally not using any 3rd party library etc?
I'm not aware about stack overflow errors caused by the mentioned change. Are you using smaller as usual JVM stack size argument to run your test?

[iloveeclipse]

Can you share the stack trace at least?

[HarryWeppner]

I am afraid that won't mean much to anyone outside. If you think it's worthwhile, I will create a reproducer. What do you think?

[iloveeclipse]

As said, reproducer is the best way to get the problem understood and maybe solved.

[HarryWeppner]

Conceptually, the test method has a signature of (Long, String) representing a target stack depth and a string that is returned as-is.

My hypothesis is that with org.eclipse.jdt.core.compiler.codegen.useStringConcatFactory=enabled, this string lands on the stack as I can break the test by first reducing the target stack depth to not cause a StackOverFlowError, and then break it again by increasing the string length that is passed around.

[HarryWeppner]

@iloveeclipse, I was about to create a reproducer but decided to upgrade to the now-released 4.34 first. To my surprise, this is no longer reproducible!

[jarthana]

String template work which involved string concatenation has been removed in 4.34. That would be my guess and with that we can close this issue.