[Java] Fory instances with different config are not independent due to codegen

[nealeu]

Search before asking

• I had searched in the issues and found no similar issues.

Version

JDK 0.14.1

Component(s)

Java

Minimal reproduce step

Create two Fory instances with codegen enabled, one with a custom serializer (in my case) and one without.

Serialize the same object with the customised class as a field within the object graph (e.g. in our case we have some BSON ObjectId refs).

What did you expect to see?

I should be able to serialise and deserialise with each successfully.

What did you see instead?

Deserialisation using the non-customised Fory fails because the codegen expects the custom Serializer which it has never been told about.

Anything Else?

I think this should be obvious in terms of isolating codegen between differently configured Fory instances/pools.

Are you willing to submit a PR?

• I'm willing to submit a PR!

[chaokunyang]

@nealeu Thanks for reporting this issue. ForyBuilder has a withName method, you can set it to a different token, so the generated Serializer cache will be be hit.

Currently we use org.apache.fory.config.Config#getConfigHash to differentiate Fory, we should create a getConfigHash and a mutable hash field in Fory class, and update it every time when users invoke register related methods. And use this new getConfigHash method to replace org.apache.fory.config.Config#getConfigHash.

@nealeu are you interested for a PR?

[nealeu]

@chaokunyang Thanks for the quick response.

I was reading https://cowtowncoder.medium.com/jackson-3-0-immutability-w-builders-d9c532860d88 in relation to other areas of our codebase, and I think Fory could potentially take the same approach, although I suspect some aspects may be more dynamic, hence your suggestion.

I'll see if any of our team are up for a PR. It would be good to get some of them the experience of paying back.

[chaokunyang]

The serializer register is more dynamic, we can't use a fully immutable builder.

[nealeu]

One of the team is having a look now.

[nealeu]

@chaokunyang Any plans to merge this with us having made the time to collaborate on a PR?