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.

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

Fix reporter for multi-thread use #7731

Merged
merged 1 commit into from Jul 12, 2019

Conversation

niboshi
Copy link
Member

@niboshi niboshi commented Jul 9, 2019

Fixes #7673

@niboshi niboshi added the cat:bug Bug report or fix. label Jul 9, 2019
Copy link
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

LGTM

reporters = _thread_local.reporters
except AttributeError:
reporters = _thread_local.reporters = []
return reporters
Copy link
Member

Choose a reason for hiding this comment

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

OK because this is faster than

return _thread_local.__dict__.setdefault('reporters', [])

if there are at least dozens of calls, in my environment.

@asi1024
Copy link
Member

asi1024 commented Jul 12, 2019

Jenkins, test this please.

@asi1024 asi1024 added the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Jul 12, 2019
@asi1024 asi1024 added this to the v7.0.0b2 milestone Jul 12, 2019
@chainer-ci
Copy link
Member

Jenkins CI test (for commit ab23f02, target branch master) succeeded!

@mergify mergify bot merged commit eabb66e into chainer:master Jul 12, 2019
@niboshi niboshi deleted the reporter-multithread branch July 16, 2019 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:bug Bug report or fix. st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Multiple Reporters cannot be active at the same time in chainer.report
4 participants