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

Eliminate thread pool mutex #97

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Conversation

nicoburns
Copy link
Collaborator

@nicoburns nicoburns commented Dec 5, 2024

This was quite straightforward (just remove the Mutex and the calls to the lock methods). And StyleThreadPool already internally contains an RwLock. Perhaps this was previously required but isn't anymore?

Reduces the diff with upstream which doesn't have this Mutex.

Servo PR:

@nicoburns
Copy link
Collaborator Author

It seems this doesn't work. Perhaps we could:

  • Pop a comment into the source code linking to this analysis
  • Move the Mutex into Servo layout_thread crates so that we don't need to maintain the diff here.

?

@jdm
Copy link
Member

jdm commented Dec 5, 2024

If it's possible to move it into Servo's layout thread code, that seems like a worthwhile change.

@nicoburns
Copy link
Collaborator Author

If it's possible to move it into Servo's layout thread code, that seems like a worthwhile change.

@jdm Implemented over in servo/servo#34480. I'll run the test suite again to check it works.

@nicoburns nicoburns added this pull request to the merge queue Dec 5, 2024
Merged via the queue into main with commit 18ca509 Dec 5, 2024
3 checks passed
@mrobinson mrobinson deleted the eliminate-thread-pool-mutex branch December 5, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants