Repository: netty/netty
Issue: FastThreadLocal slower than jdk

```
# Run complete. Total time: 00:01:21

Benchmark                                    Mode  Cnt      Score      Error  Units
FastThreadLocalBenchmark.fastThreadLocal    thrpt   20  50264,632 ± 6021,328  ops/s
FastThreadLocalBenchmark.jdkThreadLocalGet  thrpt   20  83866,710 ± 6483,523  ops/s

Benchmark                                    Mode  Cnt      Score      Error  Units
FastThreadLocalBenchmark.fastThreadLocal    thrpt   20  33525,425 ± 2633,724  ops/s
FastThreadLocalBenchmark.jdkThreadLocalGet  thrpt   20  65947,751 ± 4391,956  ops/s


```

it's slower for a number of reasons including casts, instanceof etc. but why is it here if jdk's one is faster on java 8?

Maybe the current benchmark isn't exactly fair because it doesn't use the "fast threads" to test...but still it's off in "real tests"

surprisingly the test executor is using fast threads so nevermind above line


Issue comments:
Thanks for reporting @ninja- !

@normanmaurer - assigned to you.

@Scottmitch :smiley: I am _squeezing_ every last bit of top hot methods and found lots of interesting things already.
btw. recycler.get() was something like 6% for the work and 94% for the thread local madness...I'd use something separate for write-once locals like recycler stack references. array[currentThread.getId()] is what I am going to try first. It's not that simple to do considering that thread ids could grow biggg. But it might be worth hacking hottest references like recycler stacks into FastThreadLocalThread as fields. But then it's not easy to get the fields out of FastThreadLocalThread as instanceof+cast isn't acceptable.

Instead of removing fast thread local I would rather try to fix it's problems even if it's hard.
(and then I would inject my recycler stuff to UnpaddedInternalThreadLocalMap...)

I have a commit that removes the "fast" internals(but keeps the "fast" threads, they may be useful later) while keeping the API and wrapping around java's ThreadLocal...can PR.
The thread local map is also kept for internal usecases just uses java's ThreadLocal for get and set.

```
Benchmark                                    Mode  Cnt        Score         Error  Units
FastThreadLocalBenchmark.fastThreadLocal    thrpt    4  1704783,743 ±  376972,859  ops/s
FastThreadLocalBenchmark.jdkThreadLocalGet  thrpt    4  1596349,202 ± 1182866,832  ops/s

^ on par (modified benchmark a bit, it was taking too long)
```

```
Benchmark                                    Mode  Cnt      Score      Error  Units
FastThreadLocalBenchmark.fastThreadLocal    thrpt   20  66182,502 ± 2268,703  ops/s
FastThreadLocalBenchmark.jdkThreadLocalGet  thrpt   20  64619,961 ± 4825,228  ops/s

^ original benchmark, modified netty
```

Back to Recycler now I am considering removing per-thread stacks now that that my version is more like a linked list...wdyt @normanmaurer ? Would save most of these ThreadLocal.get(). The cost of volatile should still be cheaper than threadlocal.get()
Let's see what happens if I remove thread local recyclers, make a shared one and then implement an optimistic linked queue...playing with it right now.

@ninja- please hold of for now... I want to investigate a bit more 

@ninja- 
could you run it on Java 7 as well please? Maybe it's just a change in the runtime that makes `FastThreadLocalBenchmark` regress. Also looking at the commit 7d37af5dfb33c9ca67c596, it seems like this change was justified due to improved performance in `ByteBufAllocatorBenchmark` and `RecyclableArrayListBenchmark`. So it would be interesting to see if that's still true for those two benchmarks. 

Also, your benchmark numbers seem to be very different with each run. Do you know why?

@buchgr Regarding byte buf allocator I have some other (quite simple) ideas so I'll test it as a whole
(recycler/threadlocal.get())
@buchgr are you referring to that millions-something score benchmark? I left a notice under it that it's a version that uses less iterations to test. Below is the non-modified benchmark

@ninja- please if you do any benchmarks do it with "isolated" changes and not change multiple things at the same time. 

@normanmaurer @buchgr's question is easy to test: either these two benchmarks with jdk's threadlocal will regress or not. So that's not a problem to test too

> @buchgr are you referring to that millions-something score benchmark? I left a notice under it that it's a version that uses less iterations to test. Below is the non-modified benchmark

looking at your first post, numbers seem significantly different between the runs.

@buchgr pc load I think. It would be 2x faster in every attempt to test 

@ninja- can reproduce it... looking :)

@normanmaurer I have a patch that removes thread-local recycler stack lookups(lookup once) in places where the object is either thread local already (like the PoolThreadCache) or called only inside the event loop so: PoolThreadCache(memory regions),PooledByteBufAllocator(pooled buffer objects),ChannelOutboundBuffer(outbound buffer entries). No chance to break anything and benchmarks should like this change(I'll test) and doesn't depend on other changes. Thread locals would never be free as long as Thread.currentThread() is there.

Like I said please hold off

> Am 29.10.2015 um 12:35 schrieb ninja notifications@github.com:
> 
> @normanmaurer I have a patch that removes thread-local recycler stack lookups(lookup once) in places where the object is either thread local already (like the PoolThreadCache) or called only inside the event loop so: PoolThreadCache(memory regions),PooledByteBufAllocator(pooled buffer objects),ChannelOutboundBuffer(outbound buffer entries). No chance to break anything and benchmarks should like this change(I'll test) and doesn't depend on other changes. Thread locals would never be free as long as Thread.currentThread() is there.
> 
> —
> Reply to this email directly or view it on GitHub.

@normanmaurer ...it's barely related


Pull request review comments:

Patch that resolved the issue:
diff --git a/microbench/src/main/java/io/netty/microbench/util/AbstractMicrobenchmark.java b/microbench/src/main/java/io/netty/microbench/util/AbstractMicrobenchmark.java
index 36816b16bca..c484d1cd5bb 100644
--- a/microbench/src/main/java/io/netty/microbench/util/AbstractMicrobenchmark.java
+++ b/microbench/src/main/java/io/netty/microbench/util/AbstractMicrobenchmark.java
@@ -36,8 +36,8 @@ public class AbstractMicrobenchmark extends AbstractMicrobenchmarkBase {
 
     static {
         final String[] customArgs = {
-        "-Xms768m", "-Xmx768m", "-XX:MaxDirectMemorySize=768m", "-Dharness.executor=CUSTOM",
-        "-Dharness.executor.class=AbstractMicrobenchmark$HarnessExecutor" };
+        "-Xms768m", "-Xmx768m", "-XX:MaxDirectMemorySize=768m", "-Djmh.executor=CUSTOM",
+        "-Djmh.executor.class=io.netty.microbench.util.AbstractMicrobenchmark$HarnessExecutor" };
 
         JVM_ARGS = new String[BASE_JVM_ARGS.length + customArgs.length];
         System.arraycopy(BASE_JVM_ARGS, 0, JVM_ARGS, 0, BASE_JVM_ARGS.length);
@@ -59,7 +59,6 @@ protected String[] jvmArgs() {
 
     protected ChainedOptionsBuilder newOptionsBuilder() throws Exception {
         ChainedOptionsBuilder runnerOptions = super.newOptionsBuilder();
-
         if (getForks() > 0) {
             runnerOptions.forks(getForks());
         }


Test patch: