Skip to content

Dont use ThreadDeathWatcher to cleanup PoolThreadCache if FastThreadL…#7445

Closed
normanmaurer wants to merge 1 commit into4.1from
no_thread_death_watcher_fast_thread_local_thread
Closed

Dont use ThreadDeathWatcher to cleanup PoolThreadCache if FastThreadL…#7445
normanmaurer wants to merge 1 commit into4.1from
no_thread_death_watcher_fast_thread_local_thread

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

…ocalThread with wrapped Runnable is used

Motivation:

We dont need to use the ThreadDeathWatcher if we use a FastThreadLocalThread for which we wrap the Runnable and ensure we call FastThreadLocal.removeAll() once the Runnable completes.

Modifications:

  • Dont use a ThreadDeathWatcher if we are sure we will call FastThreadLocal.removeAll()
  • Add unit test.

Result:

Less overhead / running theads if you only allocate / deallocate from FastThreadLocalThreads.

if (useRunnable) {
new FastThreadLocalThread(task).start();
} else {
new FastThreadLocalThread() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing start()? Does that mean another assertion should be added to notice?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep missed and yep let me add another assertion as well

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ejona86 good catch!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a CountdownLatch so the test would fail with a timeout if we not start these. @ejona86 PTAL again

Copy link
Copy Markdown
Member

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

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

LGTM

…ocalThread with wrapped Runnable is used

Motivation:

We dont need to use the ThreadDeathWatcher if we use a FastThreadLocalThread for which we wrap the Runnable and ensure we call FastThreadLocal.removeAll() once the Runnable completes.

Modifications:

- Dont use a ThreadDeathWatcher if we are sure we will call FastThreadLocal.removeAll()
- Add unit test.

Result:

Less overhead / running theads if you only allocate / deallocate from FastThreadLocalThreads.
@normanmaurer normanmaurer force-pushed the no_thread_death_watcher_fast_thread_local_thread branch from 132758b to 7e131b8 Compare November 28, 2017 12:43
@normanmaurer
Copy link
Copy Markdown
Member Author

Cherry-picked into 4.1 (09a05b6) and 4.0 (6507f23)

@normanmaurer normanmaurer deleted the no_thread_death_watcher_fast_thread_local_thread branch January 22, 2019 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants