Skip to content

Performance tuning: replace AtomicInteger with AtomicIntegerFieldUpdater#4305

Merged
wu-sheng merged 1 commit into
apache:masterfrom
kezhenxu94:performance/tuning
Feb 1, 2020
Merged

Performance tuning: replace AtomicInteger with AtomicIntegerFieldUpdater#4305
wu-sheng merged 1 commit into
apache:masterfrom
kezhenxu94:performance/tuning

Conversation

@kezhenxu94
Copy link
Copy Markdown
Member

Motivation:

Reduce unnecessary memory allocation in frequently used class.

Modifications:

Replace the AtomicInteger with AtomicIntegerFieldUpdater

Result:

We can save 4 bytes (for the reference to AtomicInteger itself) for each TracingContext, which saves credible memory in a high-concurrent agent service.

@kezhenxu94 kezhenxu94 added agent Language agent related. enhancement Enhancement on performance or codes labels Feb 1, 2020
@kezhenxu94 kezhenxu94 added this to the 7.0.0 milestone Feb 1, 2020
@kezhenxu94 kezhenxu94 requested a review from wu-sheng February 1, 2020 14:26
@kezhenxu94
Copy link
Copy Markdown
Member Author

kezhenxu94 commented Feb 1, 2020

If you're interested in the theory and benchmark, please refer to http://normanmaurer.me/blog/2013/10/28/Lesser-known-concurrent-classes-Part-1/ and the counterpart Chinese version is here: https://kezhenxu94.github.io/2020/01/30/Concurrent-classes-AtomicFieldUpdater/

I've also checked other classes using Atomic* and they are left AS IS because they are not concurrency-sensitive

Motivation:

Reduce unnecessary memory allocation in frequently used class.

Modifications:

Replace the `AtomicInteger` with `AtomicIntegerFieldUpdater`

Result:

We can save 4 bytes (for the reference to `AtomicInteger` itself) for each `TracingContext`, which saves credible memory in a high-concurrent agent service.
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #4305 into master will increase coverage by 0.11%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4305      +/-   ##
==========================================
+ Coverage   26.72%   26.83%   +0.11%     
==========================================
  Files        1170     1170              
  Lines       25579    25577       -2     
  Branches     3655     3655              
==========================================
+ Hits         6835     6863      +28     
+ Misses      18133    18104      -29     
+ Partials      611      610       -1
Impacted Files Coverage Δ
...walking/apm/agent/core/context/TracingContext.java 61.43% <61.53%> (+0.54%) ⬆️
...m/agent/core/remote/TraceSegmentServiceClient.java 82.35% <0%> (+1.47%) ⬆️
.../core/remote/ServiceAndEndpointRegisterClient.java 31.46% <0%> (+3.37%) ⬆️
.../agent/core/profile/ProfileTaskChannelService.java 36.78% <0%> (+8.04%) ⬆️
...alking/apm/agent/core/remote/AgentIDDecorator.java 85.71% <0%> (+17.85%) ⬆️
...ache/skywalking/apm/agent/core/jvm/JVMService.java 77.04% <0%> (+19.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c6914f...0a36c10. Read the comment docs.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Feb 1, 2020

@kezhenxu94 I think you miss an important factor, in the AtomicIntegerFieldUpdaterImpl, it is using the refactor, which causes more CPU. This is a memory-CPU tradeoff, I am not sure this is the performance tuning. In my mind, in today's production env, CPU is more valuable than the memory.

Especially, CPU is affecting latency, response time and throughput more.

@kezhenxu94
Copy link
Copy Markdown
Member Author

@kezhenxu94 I think you miss an important factor, in the AtomicIntegerFieldUpdaterImpl, it is using the refactor, which causes more CPU. This is a memory-CPU tradeoff, I am not sure this is the performance tuning. In my mind, in today's production env, CPU is more valuable than the memory.

Especially, CPU is affecting latency, response time and throughput more.

@wu-sheng as you can see, the updater is static and is only instantiated once, and reflection is only use in the instantiation to find the field offset, so the CPU cost is an one-time cost

@wu-sheng wu-sheng merged commit 360eae2 into apache:master Feb 1, 2020
@kezhenxu94 kezhenxu94 deleted the performance/tuning branch February 1, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. enhancement Enhancement on performance or codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants