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

Read write lock fully reentrant #1562

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

janbar
Copy link
Contributor

@janbar janbar commented Feb 13, 2024

  • It is fully re-entrant: read/write/read , and so it allows upgrading of reader to writer
  • No more collisions when testing recursive S
  • At least 5% faster than previous

Copy link
Collaborator

@Karry Karry left a comment

Choose a reason for hiding this comment

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

LGTM, just curious if usage of std::list would not be enough...?

@janbar
Copy link
Contributor Author

janbar commented Feb 14, 2024

LGTM, just curious if usage of std::list would not be enough...?

Hehe, using std::list you will get 5 times slower. You are right, std::list use a chain internally, but we haven't access directly to the linked nodes, previous or next: We have to use the iterator. Also it allocates a new pointer for each added element, unless we implement a dedicated allocator ... much work for a so simple thing, and not really efficient.

@janbar
Copy link
Contributor Author

janbar commented Feb 17, 2024

Please wait before merge it. I am running some tests.

Edit: using not tiled rendering, a reader thread still owns the lock in move. This behavior also existed before this commit, so there is no regression.

- It is fully re-entrant: read/write/read
- No more collisions when testing recursive S
- At least 5% faster than previous
- Uses TTAS for spin lock, instead TAS
@janbar
Copy link
Contributor Author

janbar commented Feb 17, 2024

I updated the spin lock to implement TTAS instead TAS. The performance is better again, and I cannot do better.

@Framstag , you can merge the PR as you wish.

@Framstag Framstag merged commit 79a9065 into Framstag:master Feb 17, 2024
17 of 18 checks passed
@janbar janbar deleted the read_write_lock_reentrant branch February 26, 2024 13:21
Copy link

github-actions bot commented Jun 2, 2024

🎉 This issue has been resolved in v2024.06.02.1 (Release Notes)

@github-actions github-actions bot added the released Issue has been released label Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Issue has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants