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.

Sign up for GitHub

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

Jump to bottom

Speed up the slow path of FastThreadLocal #5012

Closed
wants to merge 1 commit into from
Closed

Speed up the slow path of FastThreadLocal #5012

wants to merge 1 commit into from

Conversation

windie
Copy link
Member

@windie windie commented Mar 22, 2016

Motivation:

The current slow path of FastThreadLocal is much slower than JDK ThreadLocal. See #4418

Modifications:

  • Add FastThreadLocalSlowPathBenchmark for the flow path of FastThreadLocal
  • Add final to speed up the slow path of FastThreadLocal

Result:

The slow path of FastThreadLocal is improved.

@windie
Copy link
Member Author

windie commented Mar 22, 2016

Before 5dcc0c9

Benchmark                                            Mode  Cnt      Score     Error  Units
FastThreadLocalFastPathBenchmark.fastThreadLocal    thrpt   20  63891.724 ± 388.336  ops/s
FastThreadLocalFastPathBenchmark.jdkThreadLocalGet  thrpt   20  46496.464 ± 796.301  ops/s

Benchmark                                            Mode  Cnt      Score     Error  Units
FastThreadLocalSlowPathBenchmark.fastThreadLocal    thrpt   20  22163.357 ± 261.018  ops/s
FastThreadLocalSlowPathBenchmark.jdkThreadLocalGet  thrpt   20  46401.424 ± 780.948  ops/s

After

Benchmark                                            Mode  Cnt      Score      Error  Units
FastThreadLocalFastPathBenchmark.fastThreadLocal    thrpt   20  63397.962 ± 1205.249  ops/s
FastThreadLocalFastPathBenchmark.jdkThreadLocalGet  thrpt   20  46614.613 ±  776.756  ops/s

Benchmark                                            Mode  Cnt      Score     Error  Units
FastThreadLocalSlowPathBenchmark.fastThreadLocal    thrpt   20  30672.368 ± 269.152  ops/s
FastThreadLocalSlowPathBenchmark.jdkThreadLocalGet  thrpt   20  45984.833 ± 223.437  ops/s

windie
windie reviewed Mar 22, 2016
View reviewed changes
microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalFastPathBenchmark.java
@@ -39,19 +39,17 @@

static {
for (int i = 0; i < jdkThreadLocals.length; i ++) {
final int num = rand.nextInt();
Copy link
Member Author

Choose a reason for hiding this comment

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

Modified to put the same data to jdkThreadLocals and fastThreadLocals.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@normanmaurer normanmaurer self-assigned this Mar 22, 2016
@normanmaurer
Copy link
Member

@windie I don't see any improvements in your posted benchmark output... What I'm missing?

@windie
Copy link
Member Author

windie commented Mar 22, 2016

@normanmaurer Sorry that it's not clear. I copied the two benchmark outputs about the improvement here:

Before:

FastThreadLocalSlowPathBenchmark.fastThreadLocal    thrpt   20  22163.357 ± 261.018  ops/s

After:

FastThreadLocalSlowPathBenchmark.fastThreadLocal    thrpt   20  30672.368 ± 269.152  ops/s

@normanmaurer
Copy link
Member

@windie oh sorry I miss-read your comment before. Awesome work!

normanmaurer
normanmaurer reviewed Mar 22, 2016
View reviewed changes
common/src/main/java/io/netty/util/internal/InternalThreadLocalMap.java
}
}

public static void destroy() {
slowThreadLocalMap = null;
slowThreadLocalMap.remove();
Copy link
Member

Choose a reason for hiding this comment

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

@trustin can you comment why you not did this before when implement this ?

Copy link
Member

Choose a reason for hiding this comment

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

@normanmaurer It's to prevent slowThreadLocalMap from being initialized again after its destruction. Otherwise a user can 'resurrect' the destroyed InternalThreadLocalMap.

Copy link
Member Author

Choose a reason for hiding this comment

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

But the user can still call get to create a new slowThreadLocalMap after its destruction.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, you are right. My memory about this is fading :-)

I guess it's probably to make getIfSet() return null when slowThreadLocalMap is not created yet (or destroyed)

Copy link
Member

Choose a reason for hiding this comment

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

See here

@windie
Speed up the slow path of FastThreadLocal
50f2a44 
Motivation:

The current slow path of FastThreadLocal is much slower than JDK ThreadLocal. See netty#4418

Modifications:

- Add FastThreadLocalSlowPathBenchmark for the flow path of FastThreadLocal
- Add final to speed up the slow path of FastThreadLocal

Result:

The slow path of FastThreadLocal is improved.
@windie
Copy link
Member Author

windie commented Mar 23, 2016

Squashed.

@normanmaurer
Copy link
Member

Cherry-picked into 4.1 (3ad55eb) and 4.0 (f5b4937)

@normanmaurer normanmaurer added this to the 4.0.36.Final milestone Mar 23, 2016
@normanmaurer normanmaurer mentioned this pull request Mar 23, 2016
Closed
@windie windie deleted the fast-thread-local branch March 26, 2016 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@normanmaurer normanmaurer

Labels
improvement
Projects
None yet
Milestone
4.0.36.Final
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@windie @normanmaurer @trustin @johnou @buchgr