Skip to content

Create platform-dependent locks #657

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

Merged
merged 8 commits into from
Apr 8, 2025
Merged

Create platform-dependent locks #657

merged 8 commits into from
Apr 8, 2025

Conversation

pblazej
Copy link
Contributor

@pblazej pblazej commented Apr 3, 2025

Addresses lock-related concepts like:

  • How can we use more recent APIs (without dynamic dispatch)
  • How can we fix this kind of warning

Copy link

ilo-nanpa bot commented Apr 3, 2025

it seems like you haven't added any nanpa changeset files to this PR.

if this pull request includes changes to code, make sure to add a changeset, by writing a file to .nanpa/<unique-name>.kdl:

minor type="added" "Introduce frobnication algorithm"

refer to the manpage for more information.

@pblazej pblazej force-pushed the blaze/lock-evolution branch from 0cdd352 to 47b222e Compare April 3, 2025 14:03
@pblazej
Copy link
Contributor Author

pblazej commented Apr 3, 2025

I think the idea is generally good, maybe would be nice to run another benchmark on our new (and concrete) implementation here.

@pblazej pblazej force-pushed the blaze/lock-evolution branch from 9965a36 to 627d841 Compare April 4, 2025 12:50
@pblazej pblazej marked this pull request as ready for review April 4, 2025 13:33
@pblazej
Copy link
Contributor Author

pblazej commented Apr 4, 2025

There's no clear performance winner here (iOS ⬅️ macOS ➡️), but I think it's still beneficial to fix the warning and consume future improvements in the newer APIs.

Screenshot 2025-04-04 at 3 33 39 PM

@pblazej pblazej force-pushed the blaze/lock-evolution branch from 627d841 to 3428117 Compare April 4, 2025 13:38
@hiroshihorie
Copy link
Member

Nice. Even though there isn’t a significant gain at the moment, it’s good to transition to the safer locks.

@pblazej pblazej force-pushed the blaze/lock-evolution branch from 4c6f9db to 883913a Compare April 8, 2025 07:09
@pblazej
Copy link
Contributor Author

pblazej commented Apr 8, 2025

Nice. Even though there isn’t a significant gain at the moment, it’s good to transition to the safer locks.

👍 Haven't noticed any perf degradation with our new measurements 📐

@pblazej pblazej requested a review from hiroshihorie April 8, 2025 07:15
@pblazej pblazej merged commit 56f3962 into main Apr 8, 2025
20 checks passed
@pblazej pblazej deleted the blaze/lock-evolution branch April 8, 2025 07:30
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