Add deep copy instead of shallow copy in mappers #360
Conversation
ArtDu
force-pushed
the
default-msgpack-mapper/true-copy
branch
from
a45c752 to
c896e82
Compare
| this(); | ||
| this.valueConverters.putAll(mapper.valueConverters); | ||
| this.objectConverters.putAll(mapper.objectConverters); | ||
| this.valueConverters = mapper.valueConverters.entrySet().stream() |
There was a problem hiding this comment.
Although it's a good idea on its own, it will lead to large memory consumption growth. Remember that we are currently creating a mapper stack on each request.
Did this change come out of a recently encountered bug? I was worried about that after we made the last mappers refactoring, see my comment #300 (review). Maybe let's think about proper mapper caching this time?
Raw idea: we should define a "tagging" API for mappers (e.g. a hashcode for the whole stack) that we can use for caching the other mappers. Using such tag (hashcode) can allow caching mappers without space schema involved, but it will require each converter and mapper to provide a unique tag (hashcode).
There was a problem hiding this comment.
Did this change come out of a recently encountered bug?
It's problem in springdata, because there we use registerValueConverter per request there. We should also think about how to separate custom mappers from default ones besides caching
I haven't forgotten about:
Related issues:
Closes #166