Skip to content

Conversation

@original-brownbear
Copy link
Contributor

@original-brownbear original-brownbear commented Feb 18, 2025

Random find from debugging: using an anonymous class here doesn't compile as expected. The resulting class comes out as:

class Iterators$1 implements java.util.Iterator<T> {
  private T value;

  final java.lang.Object val$element;

  Iterators$1(java.lang.Object);

which seemingly also does not get fixed by the JIT compiler, judging by heap dumps. Lets just use a named class to clean this up and make things a bit more compact and save some heap as well here and there potentially.

Using an anonymous class here doesn't compile as expected.
The resulting class comes out as:

```
class Iterators$1 implements java.util.Iterator<T> {
  private T value;

  final java.lang.Object val$element;

  Iterators$1(java.lang.Object);
```

which seemingly also does not get fixed by the JIT compiler, judging by
heap dumps. Lets just use a named class to clean this up and make things
a bit more compact and save some heap as well here and there potentially.
@original-brownbear original-brownbear added >non-issue :Core/Infra/Core Core issues without another label labels Feb 18, 2025
@original-brownbear original-brownbear requested a review from a team as a code owner February 18, 2025 17:02
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.1.0 labels Feb 18, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me, but I don't see the extra field in my compiler output - what am I missing?

Compiled from "Iterators.java"
class org.elasticsearch.common.collect.Iterators$1 implements java.util.Iterator<T> {
  final java.lang.Object val$element;

  org.elasticsearch.common.collect.Iterators$1(java.lang.Object);
    Code:
       0: aload_0
       1: aload_1
       2: putfield      #1                  // Field val$element:Ljava/lang/Object;
       5: aload_0
       6: invokespecial #7                  // Method java/lang/Object."<init>":()V
       9: aload_0
      10: aload_0
      11: getfield      #1                  // Field val$element:Ljava/lang/Object;
      14: invokestatic  #13                 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
      17: putfield      #19                 // Field value:Ljava/lang/Object;
      20: return

  public boolean hasNext();
    Code:
       0: aload_0
       1: getfield      #19                 // Field value:Ljava/lang/Object;
       4: ifnull        11
       7: iconst_1
       8: goto          12
      11: iconst_0
      12: ireturn

  public T next();
    Code:
       0: aload_0
       1: getfield      #19                 // Field value:Ljava/lang/Object;
       4: astore_1
       5: aload_0
       6: aconst_null
       7: putfield      #19                 // Field value:Ljava/lang/Object;
      10: aload_1
      11: areturn
}

@DaveCTurner
Copy link
Contributor

Ah oops forgot -p I see it now

$ javap -p -c './server/build/classes/java/main/org/elasticsearch/common/collect/Iterators$1.class'
Compiled from "Iterators.java"
class org.elasticsearch.common.collect.Iterators$1 implements java.util.Iterator<T> {
  private T value;

  final java.lang.Object val$element;

  org.elasticsearch.common.collect.Iterators$1(java.lang.Object);
    Code:
       0: aload_0
       1: aload_1
       2: putfield      #1                  // Field val$element:Ljava/lang/Object;
       5: aload_0
       6: invokespecial #7                  // Method java/lang/Object."<init>":()V
       9: aload_0
      10: aload_0
      11: getfield      #1                  // Field val$element:Ljava/lang/Object;
      14: invokestatic  #13                 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
      17: putfield      #19                 // Field value:Ljava/lang/Object;
      20: return

  public boolean hasNext();
    Code:
       0: aload_0
       1: getfield      #19                 // Field value:Ljava/lang/Object;
       4: ifnull        11
       7: iconst_1
       8: goto          12
      11: iconst_0
      12: ireturn

  public T next();
    Code:
       0: aload_0
       1: getfield      #19                 // Field value:Ljava/lang/Object;
       4: astore_1
       5: aload_0
       6: aconst_null
       7: putfield      #19                 // Field value:Ljava/lang/Object;
      10: aload_1
      11: areturn
}

@original-brownbear
Copy link
Contributor Author

Thanks David!

@original-brownbear original-brownbear merged commit b4f84f6 into elastic:main Feb 18, 2025
17 checks passed
@original-brownbear original-brownbear deleted the cleanup-single-iter branch February 18, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants