Skip to content

Ensure ObjectCleaner will also be used when FastThreadLocal.set is used.#7534

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

Ensure ObjectCleaner will also be used when FastThreadLocal.set is used.#7534
normanmaurer wants to merge 1 commit into4.1from
fast_thread_local_cleaner_Set

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

Motivation:

e329ca1 introduced the user of ObjectCleaner in FastThreadLocal but we missed the case to register our cleaner task if FastThreadLocal.set was called only.

Modifications:

  • Use ObjectCleaner also when FastThreadLocal.set is used.
  • Add test case.

Result:

ObjectCleaner is always used.

Motivation:

e329ca1 introduced the user of ObjectCleaner in FastThreadLocal but we missed the case to register our cleaner task if FastThreadLocal.set was called only.

Modifications:

- Use ObjectCleaner also when FastThreadLocal.set is used.
- Add test case.

Result:

ObjectCleaner is always used.
@normanmaurer
Copy link
Copy Markdown
Member Author

@zpencer PTAL

Copy link
Copy Markdown
Member

@Scottmitch Scottmitch left a comment

Choose a reason for hiding this comment

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

few comments but generally lgtm

if (value != InternalThreadLocalMap.UNSET) {
set(InternalThreadLocalMap.get(), value);
InternalThreadLocalMap threadLocalMap = InternalThreadLocalMap.get();
boolean alreadySet = threadLocalMap.isIndexedVariableSet(index);
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.

consider making an overload of set which returns alreadySet because it already knows this information. it could be private static.

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.

This is not alway true... let me just merge it as it is for now

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.

PR #7541 is what I had in mind

*/
@SuppressWarnings("unchecked")
public final V get() {
final InternalThreadLocalMap threadLocalMap = InternalThreadLocalMap.get();
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.

was removing final intentional and for stylistic reasons?

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.

yeah I added it before when I needed it as I used it in the Runnable but as I moved this to an extra method I not need it anymore

@normanmaurer
Copy link
Copy Markdown
Member Author

Cherry-picked into 4.1 (e004b4a) and 4.0 (09484de)

@normanmaurer normanmaurer deleted the fast_thread_local_cleaner_Set branch December 22, 2017 06:12
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants