Unnecessary eager creation of Json and XML mappers in hibernate-orm extension

[mariofusco]

Description

In order to implement this feature, the hibernate-orm extension eagerly triggers the creation of Json and XML mappers here.

This eager initialization is also the cause of the issue reported here. As discussed with @lucamolteni, I can probably fix that problem in a different and more robust way (and I'm going to try to implement his suggestion right now), but in all cases it would be better I believe that it would be better (also to improve the startup time) to avoid those eager initializations if possible.

[quarkus-bot]

/cc @gsmet (hibernate-orm)

[lucamolteni]

Lazily load the serializer will probably lead to faster startup time / better memory usage, but I don't know yet how much it'd impact

[geoand]

Anything we can do lazily WRT Jackson helps

[gsmet]

Frankly, I still think it's very brittle and dangerous to share the ObjectMapper between Hibernate ORM and the REST layer.

You might want to tweak your (de)serialization for the REST layer, whereas the (de)serialization format for the ORM one needs to be extremely stable (or you might have to migrate the existing data).

/cc @yrodiere @marko-bekhta

[yrodiere]

Lazily load the serializer will probably lead to faster startup time / better memory usage, but I don't know yet how much it'd impact

Asssuming the serializer is actually useful, it would not, because when we measure startup time and RSS, we don't cheat: we measure it all after the first HTTP response. So all lazy initialization already should have happened by then.

Now, if you're talking about an app where the serializer is never/rarely used -- which I guess could be the case for XML, at least? -- then sure, there could be a gain. Like you, I don't know how much.

And of course, lazy initialization can still be useful for performance-unrelated matters.

Frankly, I still think it's very brittle and dangerous to share the ObjectMapper between Hibernate ORM and the REST layer.

You might want to tweak your (de)serialization for the REST layer, whereas the (de)serialization format for the ORM one needs to be extremely stable (or you might have to migrate the existing data).

I am @yrodiere and I approve this message.

Though I'm not sure it would change anything to the issue being discussed.

[gsmet]

Though I'm not sure it would change anything to the issue being discussed.

If we are using different mappers, I would expect the eager initialization of the ORM ones not to be a problem for the REST ones.

But maybe I misunderstood the issue.

[yrodiere]

Though I'm not sure it would change anything to the issue being discussed.

If we are using different mappers, I would expect the eager initialization of the ORM ones not to be a problem for the REST ones.

But maybe I misunderstood the issue.

My understanding is that we don't want to have the object mappers initialized eagerly because it's costly and potentially pointless -- see #47594 (comment). IMO, the JSON object mapper for REST is likely to always be needed, so I assume the problem was mostly around the XML one.

In any case, if we add two more eagerly initialized object mappers just for Hibernate, I suspect we're not solving the problem, even making it worse?

[lucamolteni]

Now, if you're talking about an app where the serializer is never/rarely used -- which I guess could be the case for XML, at least? -- then sure, there could be a gain. Like you, I don't know how much.

Exactly, I was referring to the case where entities are not serialised, from a quick debugging session with Mario it seems like it's always initialised as this configuration is present by default

[marko-bekhta]

if (puDefinition.hasEnititesWithJsonMapping()) {
    if (puDefinition.getJsonMapperCreator().isPresent()) {
        cfg.put(AvailableSettings.JSON_FORMAT_MAPPER, puDefinition.getJsonMapperCreator().get().create());
    } else
        throw new IllegalStateException("Persistent unit %s contains entities with JSON properties, but no JSON mapper is available. Consider adding .... "
    }
}

🙈 ? (maybe puDefinition.hasEnititesWithJsonMapping() could be easier to do with hibernate-models and ORM 7?)

[yrodiere]

if (puDefinition.hasEnititesWithJsonMapping()) {
if (puDefinition.getJsonMapperCreator().isPresent()) {
cfg.put(AvailableSettings.JSON_FORMAT_MAPPER, puDefinition.getJsonMapperCreator().get().create());
} else
throw new IllegalStateException("Persistent unit %s contains entities with JSON properties, but no JSON mapper is available. Consider adding .... "
}
}

🙈 ? (maybe puDefinition.hasEnititesWithJsonMapping() could be easier to do with hibernate-models and ORM 7?)

Exactly. Currently that would be dodgy to implement, and we really don't need one more thing like that.

I'd much rather take advantage of lazy initialization if that solves the problem, sparing us from processing Hibernate ORM metadata.