Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use singleton ObjectMapper #213

Merged
merged 1 commit into from
May 24, 2024
Merged

Conversation

jjaakola-aiven
Copy link
Contributor

Reduces some memory footprint as single reused object mapper is used instead of creating new for each output stream, key, header and value serializer.

@jjaakola-aiven jjaakola-aiven requested review from a team as code owners November 22, 2023 13:59
Copy link
Contributor

@jeqo jeqo left a comment

Choose a reason for hiding this comment

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

Thanks @jjaakola-aiven!

Overall LGTM, left some comments.


class KeyBuilder implements OutputFieldBuilder {

private final ObjectMapper objectMapper = new ObjectMapper();
Copy link
Contributor

Choose a reason for hiding this comment

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

mental note; changing from plain object mapper to one with a exact big decimal. I don't think it may be relevant unless record.key() contains an object with big decimal values


class ValueBuilder implements OutputFieldBuilder {

private final ObjectMapper objectMapper = new ObjectMapper();
Copy link
Contributor

Choose a reason for hiding this comment

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

same mental note as for KeyBuilder

Reduces some memory footprint as single reused object mapper is used
instead of creating new for each output stream, key, header and value
serializer.
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-use-singleton-objectmapper branch from 6ffe433 to b29c779 Compare November 22, 2023 14:51
@jjaakola-aiven jjaakola-aiven requested a review from jeqo November 22, 2023 14:53
Copy link
Contributor

@snuyanzin snuyanzin left a comment

Choose a reason for hiding this comment

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

lgtm from my side

@jeqo jeqo merged commit ec5fa8f into main May 24, 2024
8 checks passed
@jeqo jeqo deleted the jjaakola-aiven-use-singleton-objectmapper branch May 24, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants