Skip to content

KAFKA-10012 Reducing memory overhead associated with strings in Metri…#8684

Merged
hachikuji merged 3 commits into
apache:trunkfrom
navina:apache-metric-name-optimization
Jun 8, 2020
Merged

KAFKA-10012 Reducing memory overhead associated with strings in Metri…#8684
hachikuji merged 3 commits into
apache:trunkfrom
navina:apache-metric-name-optimization

Conversation

@navina

@navina navina commented May 18, 2020

Copy link
Copy Markdown

SelectorMetrics has a per-connection metrics, which means the number of MetricName objects and the strings associated with it (such as group name and description) grows with the number of connections in the client. This overhead of duplicate string objects is amplified when there are multiple instances of kafka clients within the same JVM.
This patch address some of the memory overhead by making metricGrpName a constant and introducing a new constant perConnectionMetricGrpName. Additionally, the strings for metric name and description in createMeter have been interned since there are about 8 types of meter metric for every single client.

@ijuma

ijuma commented May 18, 2020

Copy link
Copy Markdown
Member

Thanks for the PR. Java's built in string interning mechanism is known to have issues. Not sure we want to do that. Maybe we can remove that part of the change from this PR?

@navina

navina commented May 18, 2020

Copy link
Copy Markdown
Author

@ijuma What issues with intern() are you referring to? I know that there can be a performance hit when there are a lot of intern strings. I believe string interning mechanism has been improved in the later versions of java such as jdk8 / 9.
I would like to understand the concern better before removing the string interning showed here.
Thanks for the quick feedback!

@ijuma

ijuma commented May 30, 2020

Copy link
Copy Markdown
Member

@navina it's a native call (implying JNI overhead) and somewhat inflexible. We'd want to add a JMH benchmark to understand the full impact.

@navina
navina force-pushed the apache-metric-name-optimization branch from 899a8ad to 8d3db0d Compare June 1, 2020 22:10
@navina

navina commented Jun 2, 2020

Copy link
Copy Markdown
Author

@ijuma I have removed the intern() calls. Please take a look. Thanks!

@hachikuji hachikuji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Will let @ijuma do a final pass.

@ijuma

ijuma commented Jun 7, 2020

Copy link
Copy Markdown
Member

@hachikuji feel free to merge, my main concern was the intern calls that have been removed. We should update the commit message to remove the bit about interning before merging.

@hachikuji
hachikuji merged commit 7777dc8 into apache:trunk Jun 8, 2020
hachikuji pushed a commit that referenced this pull request Jun 8, 2020
`SelectorMetrics` has a per-connection metrics, which means the number of `MetricName` objects and the strings associated with it (such as group name and description) grows with the number of connections in the client. This overhead of duplicate string objects is amplified when there are multiple instances of kafka clients within the same JVM.  This patch addresses some of the memory overhead by making `metricGrpName` a constant field and introducing a new field `perConnectionMetricGrpName`. 

Reviewers: Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
ijuma added a commit to confluentinc/kafka that referenced this pull request Jun 8, 2020
Conflicts:
* build.gradle: take upstream changes regarding heap memory
configuration for the build.

* apache-github/trunk: (33 commits)
  MINOR: fix HTML markup (apache#8823)
  KAFKA-10012; Reduce overhead of strings in SelectorMetrics (apache#8684)
KAFKA-9216: Enforce internal config topic settings for Connect workers
during startup (apache#8270)
  KAFKA-10097: Internalize checkpoint data (apache#8820)
KAFKA-10033: Throw UnknownTopicOrPartitionException when modifying a
non-existent topic's config
MINOR: improve code encapsulation between StreamThread and TaskManager
(apache#8819)
  Fixing KAFKA-10094 (apache#8797)
KAFKA-9851: Revoking Connect tasks due to connectivity issues should
also clear the running assignment (apache#8804)
KAFKA-9840; Skip End Offset validation when the leader epoch is not
reliable (apache#8486)
  HOT_FIX: Update javadoc since imports added (apache#8817)
  KAFKA-8011: Fix flaky RegexSourceIntegrationTest (apache#8799)
KAFKA-9570: Define SSL configs in all worker config classes, not just
distributed (apache#8135)
KAFKA-10111: Make SinkTaskContext.errantRecordReporter() a default
method (apache#8814)
KAFKA-10110: Corrected potential NPE when null label value added to
KafkaMetricsContext (apache#8811)
MINOR: Change the order that Connect calls `config()` and `validate()`
to avoid validating if the required ConfigDef is null (apache#8810)
MINOR: fix backwards incompatibility in JmxReporter introduced by
KIP-606
  MINOR: Fix javadoc warnings (apache#8809)
  KAFKA-9441: Improve Kafka Streams task management (apache#8776)
  fix the broken links of streams javadoc (apache#8789)
KAFKA-10040; Make computing the PreferredReplicaImbalanceCount metric
more efficient (apache#8724)
  ...
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