Skip to content

[improve][broker] Remove uneffective solution for reducing GC pressure#20428

Merged
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-replace-uneffective-gc-pressure-mitigation
May 31, 2023
Merged

[improve][broker] Remove uneffective solution for reducing GC pressure#20428
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-replace-uneffective-gc-pressure-mitigation

Conversation

@lhotari

@lhotari lhotari commented May 29, 2023

Copy link
Copy Markdown
Member

Motivation

The PublisherStatsImpl and ConsumerStatsImpl classes contain an uneffective solution for reducing GC pressure.
This code is confusing and doesn't help. Instead, ordinary fields are used.
The solution was originally introduced by #1480.

Modifications

  • remove the uneffective solution
  • add a solution for reducing unnecessary object creation by sharing the client source address and port String object across all consumer and publisher stats on a single connection.

Additional Context

The Guava Interner based solution in #20432 could be a way to reduce GC pressure in a more effective way if there is a need for that.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added the type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use label May 29, 2023
@lhotari lhotari added this to the 3.1.0 milestone May 29, 2023
@lhotari lhotari self-assigned this May 29, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label May 29, 2023
@lhotari

lhotari commented May 29, 2023

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@JsonIgnore
private int producerNameLength;

private String producerName;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How can these changes affect JSON serde code path? I don't know if we pass or persist these classes via JSON somewhere.

If no, @JsonIgnore can be redundant. If so, adding new serialize field may affect especially this is a class under pulsar-common.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The former @JsonIgnore annotations were needed because of the extra ordinary solution to store String values. They aren't needed anymore.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is no change in JSON serialization and deserialization from an external viewpoint.

@lhotari lhotari requested a review from tisonkun May 31, 2023 13:21
@lhotari lhotari merged commit b336074 into apache:master May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants