Ebean bytecode transformation of records being questioned (unfairly)

[rbygrave]

Summary

A summary explanation of what this is about is now at: https://ebean.io/docs/well-thats-interesting/embedded-id

---

The bytecode transformation that ebean uses for java record is being questioned aggressively and unfairly on stack overflow.

When you get into the actual details, it turns out that ebean is doing nothing wrong but ebean is taking a reputational hit by some comments that assume that ebean isn't handling customisation of record types. THIS IS NOT THE CASE.

In terms of bytecode transformation, record types are in fact the easy case and there isn't bytecode in record types that ebean is not treating properly. We have no evidence that ebean is doing anything wrong at all ... but ebean is taking heavy criticism.

This heavy criticism is being delivered by Brian Goetz who is the Java Language Architect so this makes this a massive issue. As I see it, currently Brian is taking the approach of assuming that ebean is incorrectly enhancing record types and I think this is really unfair. Given Brian's role in the Java community his words have extreme weight and will create significant reputational damage to ebean and possbly already has done so.

Ebean isn't perfect and we are open to criticism but it seems really unfair for that criticism to come without any evidence of doing anything wrong. There seems to be an assumption of guilt.

If you could please have a look at: https://stackoverflow.com/questions/73377190/bytecode-transforming-record-class-to-be-mutable ... have a good read and look at the details that I have supplied Brian.

The really good news here is that having gone through this and looked really really closely again at the bytecode transformation on record classes that I am confident the enhancement is doing the right thing. This confidence comes from Brian's last questions around dealing with customisation of records - this is because these same issues appear with mutable types (non-records). To a large extent record types are a really simple case for the enhancement agent.

So please have a good read of that stack overflow question and if you can offer support that would be good. As I say in SO, this is a super serious issue that we must take really really seriously.

Thanks, Rob.

[rbygrave]

An update:

On the positive:
I'm now super confident that ebean transformation isn't doing anything wrong wrt record type

On the negative:
I've just had a direct conversation with Brian Goetz. He expressed a concern about record equals() semantics. I explained how that isn't a new thing to deal with and wouldn't be a problem etc but he really isn't softening his stance or words. I feel like he is not interested in the details and I feel like he did not give me a chance to explain properly.

The words aimed at Ebean are highly damaging like: "those who engage in it should be shamed out of the community". Ebean is going to take a massive hit forever going forward because Brian thinks we are "pissing in the pool".

The thing that really hurts here is that I'm really really confident that the bytecode transformation is doing nothing wrong but we don't seem to be getting a chance to defend ourselves. We are not being offered an opportunity to explain accurately how various cases are dealt with such that we still honor the semantics of record type. The approach taken is one of presumed guilt and then not listening to the defense arguments.

Perhaps part of the issue is that we are in the ORM business. As such, we are a long way away from if/then/else helloWorld() code. In the ORM business we are deep into bytecode transformation because we are looking for good ways to deal with issues like interception, partial objects, lazy loading, dirty checking etc. We are in the rabbit hole and bytecode details matter here and this scares Brian and he doesn't have faith that we are up to the task and he does not want people to do down here into these details.

For Ebean in the ORM business, we go into bytecode transformation because it really does enable us to do what we need to do and it really simplifies our world. It simplifies our dealing with issues like interception, partial objects, lazy loading, dirty checking. Ebeans bytecode transformation is a big reason why ebean is simpler and smaller than the other ORMs that we compare ourselves to. We go down into these details for really good reasons so solve hard problems. When we do this we must be excellent, accurate and care about bytecode details.

The annoying thing to me when I'm looking at the details on this is that honoring the semantics of record type isn't actually hard or even new to ebean bytecode transformation. Record types actually present nothing new or need any special treatment as far as ebean bytecode transformation is concerned (1).

A vote for Ebean/Rob at https://stackoverflow.com/questions/73377190/bytecode-transforming-record-class-to-be-mutable ... could help. Any help and support is greatly appreciated.

Cheers, Rob.

(1) Caveat, well if we detect a type is a record we can choose not to add the _ebean_identity field. The reason being that with record types a equals/hashCode implementation must always exist. Currently the transformation always adds the _ebean_identity field and for record types that field is completely redundant and not used at all - a bit wasteful, I've logged that as a bug and fixed it. This is in fact the only special treatment we apply for record type.

[vnnv]

Rob, just my 2 cents. Ebean as a project gave me (and still give me) so much help for my projects. Brian Goetz may be a "god" but gods are making mistakes too. So I just do not give a **** for his opinion. Keep the good work :)

[rbygrave]

The plan: Lets have a review session

I will look to organise a session for people who are interested in the details around this issue - an Ebean council if you like. I know Eddie is on holiday so we need to wait a few days at least. If you want to participate maybe reply to this or something.

Agenda:

• The questions Brian has posed.
• What record type bytecode looks like
• What the enhancement does and why it does it.
• [Maybe]: What actually occurs when we deal with customisation code in constructor, equals/hashCode, accessors, toString etc (it's actually pretty simple once you understand what its doing).
• What path do people want to take going forward? (Which might be odd, we don't actually currently do anything to detect record type or treat it any differently to non record type)

Notes:

• ebean agent doesn't give record type any special treatment.
• there really does appear to be no actual technical issue with the bytecode transformation
• I suggest that this issue is getting political

[rbygrave]

I've updated my answer and still super keen for upvotes on https://stackoverflow.com/questions/73377190/bytecode-transforming-record-class-to-be-mutable ... any support there would be great.

I'm planning to better document:

• Why we use bytecode transformation
• What the bytecode transformation does
• How you can view and see the bytecode transformation (add a ebean.mf with synthetic: false - & mvn process-test-classes)
• What records actually look like in bytecode terms
• Why records present as nothing new to ebean

[GlenKPeterson]

Perspective
What are the programmer's expectations, storing an immutable Record in a traditionally mutable SQL row? Are they reasonable? What are the better solutions to this situation?

Discussion
I confess, I started skimming after a while, so pardon me if I missed this.

The SQL database model has traditionally been one of mutation-in-place. I picture a retail store shelf stocked with products. When someone takes a product off the shelf, the contents are mutated in place: items--. When restocked: items++. Why would a programmer model that as something forever unchanging (a Java Record)?

I have stuffed some relatively unchanging things into SQL database tables, but I did it knowing I was putting a square peg into a round hole. There is some immutable/functional saying, "Some evil is necessary in order to get work done." Heck, if I understand this right, Goetz is putting an immutable Record onto a JVM that does not enforce those guarantees!

Goetz is brilliant and has done a better job leading Java than I ever could. But Java has no moral high ground to stand on where immutability is concerned. Scala assumed it in 2004, Clojure in 2007, and Kotlin in 2011. Java released records in 2020. Java still has no interfaces to represent unmodifiable collections. But I digress.

Rob, you and Ebean are awesome. As always, thank you for that. I know Goetz himself responded, but the question is a month old and only has 9 up-votes. Bytecode manipulation is kind of evil in general, yet it's the least-bad solution to ORM in Java. What do other JVM ORM's do that is so much better than the way Ebean handles this? I bet they do the same thing.

My lens for the technical aspect of this discussion is the intersection of, "What is the Java programmer's intention and expectation in this scenario?" and "What is practical to provide in this situation?" If the expectation is nonsensical, then I'd favor a good error message and/or documentation as the solution. You shouldn't need to explain or defend yourself or Ebean beyond that.

[rbygrave]

storing an immutable Record

The case is more for concatenated primary keys so @EmbeddedId / @IdClass.

People want to do:

@Embeddable
public record UserRoleId(Integer userId, String roleId) { }

Rather than:

  @Embeddable
  public static class UserRoleId {

    private Integer userId;
    private String roleId;

    public Id(Integer userId, String roleId) {
      this.userId = userId;
      this.roleId = roleId;
    }
    
    public Integer userId() { return userId; }

    public String roleId() { return roleId; }

    @Override
    public boolean equals(Object o) {
      if (this == o) return true;
      if (o == null || getClass() != o.getClass()) return false;

      UserRoleId that = (UserRoleId) o;
      return Objects.equals(userId, that.userId) && Objects.equals(roleId, that.roleId);
    }

    @Override
    public int hashCode() {
      return Objects.hash(userId, roleId);
    }
  }

For this case record type is an exact match for what we need for an @EmbeddedId - semantics of equals/hashCode and everything. It's actually perfect for this case.

[rbygrave]

Bytecode manipulation is kind of evil in general

Totally fair :) The more optimistic view is that it's a form of meta programming where we have a program (ebean-agent) modifying and generating code for us so that we don't have to. The evil part and the part that quite frankly ebean has done badly with is that its hard to see and very poorly explained ... so it presents to most people as unknown magic. One good outcome from this is that we are going to have to put effort into showing people how they actually see the resulting code and what it does and why. In theory if we do that well it will go more from "evil magic" to more "meta programming".

yet it's the least-bad solution to ORM in Java.

Well yes and all the ORMs we compare ourselves to do this and for really good reasons - it's a great way to simplify complex problems for us. Bytecode transformation is a thing explicitly supported by java of course and it just means these transformers live in that world between javac and the jvm. Other areas like APM Application performance monitoring are also big in this space.

What do other JVM ORM's do that is so much better than the way Ebean handles this? I bet they do the same thing.

Actually of the 3 others I looked at none of the them handle it.

There is a slightly related topic around "removing the restriction for default constructors for entity beans". Only 1 of them handles that case at the moment. I have a few theories on why they are so behind in this area but ultimately it does mean we are leading here for better or worse.

[GlenKPeterson]

Actually of the 3 others I looked at none of the them handle it.

No good deed goes unpunished?

[GlenKPeterson]

So the use case is Java records as code generators. They are not expecting immutability. I expect people are not interested in the details and probably didn't read more than enough of your arguments to judge the tone. I don't think the disparagement was originally directed at ebean. More that Ebean got caught in the crossfire of some fairly unrelated opinions.

I don't see any root issue that needs fixing. The stack overflow question seems kinda sleepy. Maybe ignore it and it will go away? Happy to delete my comment there if it has any chance of being a net negative.

[rbygrave]

Well I think to some extent the best "next step" is to get more people to understand and look at the details. That really means we need to produce good documentation on all the details around record but also on all the enhancement in general - really peel back the covers. We need this so that it's easier for people to look closely and understand the details and hence where there might be issues (and Brian's point is that we only need 1 per say).

I've had some great help from Roland and good input from a couple of others which has been great. There are a bunch of details that ideally get full and proper investigation if only to get the educational benefit.

We need to make some really nice things more explicit: The "bigger picture design" aspect of the enhancement (interception, dirty checking etc) + @Transaction with location profiling + query beans enhancement (also has location profiling).

We probably need to make sure we slow down, get the details as clear as we can (more eyes on the details), understand all the options available etc. To some extent we can use this as a trigger to look at the broader things around enhancement.

A lot of good can come out of this :)

[rbygrave]

I've put up a summary of what this is all about at: https://ebean.io/docs/well-thats-interesting/embedded-id

Hopefully that is a bit easier to follow than the SO post. Keen and happy to take any feedback on that page.

Cheers, Rob.

[GlenKPeterson]

I think it's more effective to say something very clearly and simply once, than to repeat yourself. "The less you write, the more of it people will read." It also makes it easier to better organize your points. Suggested changes:

JPA and Ebean use @EmbeddedId or @IdClass to map concatenated primary keys. For both, the Primary key is modeled as an @embeddable. Primary keys are generally unchanging and best represented by an immutable class.

Great example!

It used to be the developer's responsibility to correctly implement equals() and hashCode() methods for an @embeddable that maps a primary key. The implementations must include all components (partId and brandId in this case). The ORM cannot fix your mistakes here, but most IDEs could generate these methods for you.

Adding a default constructor

Other JPA implementations (not Ebean) require a default constructor. This forces Java classes to look much less immutable:

Your bullet points and example look great.

Java Shorthand

Java 14 records are immutable with correct equals() and hashCode() implementations. They provide a perfect shorthand for mapping a concatenated primary key with @embeddable in Ebean.

@Embeddable
public record PartBrandKey(long partId, long brandId) { }

Kotlin Shorthand

@Embeddable
data class PartBrandKey(val partId: Long, val brandId: Long)

Java Language vs. JVM Bytecode

Annotations like @Embeddable or @Transactional are markers for transformation to be applied to the bytecode generated by compiling the annotated class or method. Ebean bytecode transformation applies equally to Java and other JVM languages like Kotlin, Scala, and Groovy. Java 14 record types are an important (and cool) step forward for the Java language, but the bytecode they compile to isn't much different from immutable Java non-record classes or Kotlin data classes. No changes were necessary to the Ebean code in order to support Java 14 records.

Would mapping-embeddedid-classes be better url slug for this? It seems like this should be stuck into the standard documentation somewhere.

I just noticed you use @EmbeddedId, @Embeddable and some other similar annotations here. I don't know the difference or when to use one or another, so cannot comment on or review that aspect.

You might consider deleting most of your stack overflow posts on this issue and providing a link to this summary instead.

Hope that helps!

[GlenKPeterson]

Question: could I have suggested these changes with a pull request to some .MD files somewhere? I didn't immediately see where. A lot of other projects' documentation now has a "Fix this page on github" link on every page. I think I would have contributed more to the docs if it was more obvious and easy.

[rbygrave]

Awesome thanks!!! I've applied your edits with a couple of tweaks myself.

I have added a section on Java Memory Model which I now realize isn't something we talk about now (probably due to 64bit OS, java.util.concurrent & CAS operations ... e.g. I can't remember the last time I used volatile myself).

consider deleting most of your stack overflow posts on this issue and providing a link to this summary instead.

I will look to add the link at the top of the SO answer. I don't think it's great form to remove content or heavily edit the existing content (even if it is way too much detail for people to consume).

mapping-embeddedid-classes be better url slug for this?

Yes, needs a better url. I can easily move it etc.