Simplify conjure object toString implementation

[carterkozak]

Avoid using unnecessary stringbuilders, in jdk8 javac generates
this code for us. Using a compiler target beyond jdk8 we can
take advantage of jep 280.

After this PR

==COMMIT_MSG==
Simplify conjure object toString implementation
==COMMIT_MSG==

[changelog-app]

Generate changelog in changelog/@unreleased

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

Simplify conjure object toString implementation

Check the box to generate changelog(s)

• Generate changelog entry

[markelliot]

since the described JEP is to translate the bytecode generation to the thing we were producing anyway, is this change actually that beneficial?

[carterkozak]

Before jep280 java generated bytecode matching what we generated. Newer versions implementing jep280 generate simpler bytecode which can be optimized beyond stringbuilder. I can post example bytecode when I’m near a keyboard (likely won’t make it today)

[carterkozak]

Using IntegerExample for simplicity.

Methodology

Clean and rebuild with an updated target version, and read bytecode using:

javap -c ./conjure-java-core/build/classes/java/integrationInput/com/palantir/product/IntegerExample.class

Before this change (develop)

  public java.lang.String toString();
    Code:
       0: new           #7                  // class java/lang/StringBuilder
       3: dup
       4: ldc           #8                  // String IntegerExample
       6: invokespecial #9                  // Method java/lang/StringBuilder."<init>":(Ljava/lang/String;)V
       9: bipush        123
      11: invokevirtual #10                 // Method java/lang/StringBuilder.append:(C)Ljava/lang/StringBuilder;
      14: ldc           #11                 // String integer
      16: invokevirtual #12                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      19: ldc           #13                 // String :
      21: invokevirtual #12                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      24: aload_0
      25: getfield      #3                  // Field integer:I
      28: invokevirtual #14                 // Method java/lang/StringBuilder.append:(I)Ljava/lang/StringBuilder;
      31: bipush        125
      33: invokevirtual #10                 // Method java/lang/StringBuilder.append:(C)Ljava/lang/StringBuilder;
      36: invokevirtual #15                 // Method java/lang/StringBuilder.toString:()Ljava/lang/String;
      39: areturn

With this change, targeting java 8

Here we see the old (pre jep280) compiler optimization resulting in the code we used to generate. Note this is a bit more compact than the old implementation due to this PR combining delimiter and field name strings.

  public java.lang.String toString();
    Code:
       0: new           #7                  // class java/lang/StringBuilder
       3: dup
       4: invokespecial #8                  // Method java/lang/StringBuilder."<init>":()V
       7: ldc           #9                  // String IntegerExample{integer:
       9: invokevirtual #10                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      12: aload_0
      13: getfield      #3                  // Field integer:I
      16: invokevirtual #11                 // Method java/lang/StringBuilder.append:(I)Ljava/lang/StringBuilder;
      19: bipush        125
      21: invokevirtual #12                 // Method java/lang/StringBuilder.append:(C)Ljava/lang/StringBuilder;
      24: invokevirtual #13                 // Method java/lang/StringBuilder.toString:()Ljava/lang/String;
      27: areturn

With this change, targeting java 11

This is much simpler, we're giving the runtime a great deal more freedom to presize the resulting string rather than being prescriptive about the implementation details.

  public java.lang.String toString();
    Code:
       0: aload_0
       1: getfield      #2                  // Field integer:I
       4: invokedynamic #6,  0              // InvokeDynamic #0:makeConcatWithConstants:(I)Ljava/lang/String;
       9: areturn

[markelliot]

Thanks. Should there be a corresponding errorprone and refaster rule?

[carterkozak]

Should there be a corresponding errorprone and refaster rule?

That's a good idea. Filed palantir/gradle-baseline#831

[svc-autorelease]

Released 4.4.0

[carterkozak]

@markelliot automated fix is here, if you're interested: palantir/gradle-baseline#832