search

smallrye / smallrye-graphql

Implementation for MicroProfile GraphQL
Apache License 2.0
155 stars 89 forks source link

Get rid of the EventEmitter ThreadLocal #2133

Closed gsmet closed 1 month ago

gsmet commented 1 month ago

Part of the reasons why QuarkusClassLoader instances are kept around is the ThreadLocal in EventEmitter.

I'm not exactly sure why you are using a ThreadLocal here? The contract of the eventing services is not thread safe? I would expect them to be thread safe but I suppose there's a good reason.

In any case, I experimented with removing it and it seems to improve things quite significantly.

Now I'm not entirely sure why this was implemented in the first place so I open the PR more for discussion than to get merged right away...

/cc @phillip-kruger @cescoffier @jmartisk

gsmet commented 1 month ago

Same as the other ones, ideally, it would be nice to have it in Quarkus relatively fast. @brunobat is fighting with a PR (partly) due to these leaks.

jmartisk commented 1 month ago

Yeah I'm on it