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

Use thread local storage for ChiselContext. #1404

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

ucbjrl
Copy link
Contributor

@ucbjrl ucbjrl commented Apr 7, 2020

I tried to come up with a self-contained test for this, but was unable to.
I tested it by using this to build/run the bundleStackCrash branch of chisel-template.

Related issue: #1334

Type of change: bug report

Impact: no functional change

Development Phase: implementation

Release Notes
Use thread local storage for ChiselContext.

@ucbjrl ucbjrl added this to the 3.3.0 milestone Apr 7, 2020
@ucbjrl ucbjrl requested review from ducky64 and jackkoenig April 7, 2020 21:13
@ucbjrl ucbjrl requested a review from a team as a code owner April 7, 2020 21:13
Copy link
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

This looks sane for now, but the implementation can cause pitfalls in the future if we want to pursue multithreading for performance (since context data from the parent context is discarded). But since that's not a goal for now (or perhaps even in the near future), this is fine, and if it works, should address the parallel tests use case.

@ucbjrl ucbjrl added Backport Automated backport, please consider for minor release Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. labels Apr 8, 2020
@mergify mergify bot merged commit e55c00f into master Apr 8, 2020
@ucbjrl ucbjrl added Backport Automated backport, please consider for minor release and removed Backport Automated backport, please consider for minor release labels Apr 8, 2020
ucbjrl added a commit that referenced this pull request Apr 9, 2020
ucbjrl added a commit that referenced this pull request Apr 10, 2020
@ucbjrl ucbjrl deleted the threadlocalChiselcontext branch April 10, 2020 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants