Take a per-object lock in objc_setAssociatedObject, and only to install - #409
Open
DTW-Thalion wants to merge 2 commits into
Open
Take a per-object lock in objc_setAssociatedObject, and only to install#409DTW-Thalion wants to merge 2 commits into
DTW-Thalion wants to merge 2 commits into
Conversation
setReference searched the reference list for the key, then took the striped lock around a block whose only statement was a test for that search having failed. Replacing the value of a key already present therefore acquired and released the lock around nothing: the value and the policy are written after that block, outside the lock, guarded separately and only when either policy is atomic. The search now decides whether the lock is taken at all, and a second search runs under it, so two threads that both find the key absent no longer install two references for one key. One association per thread on distinct objects, 32 cores, ns per objc_setAssociatedObject: 14.1 to 4.4 at 1 thread, 66.5 to 4.5 at 4, 112.6 to 4.7 at 8, 224.0 to 9.2 at 16 and 461.0 to 12.3 at 24. objc_getAssociatedObject is unchanged at 2.7. The suite passes, 198 of 198. The unsynchronized read of a key in findReference is unchanged; it is what the search outside the lock already did.
lock_for_pointer discards the low 8 bits of the pointer, so two reference lists within 256 bytes of each other take the same lock whatever the size of the striped table. The reference list now carries a ThinLock of its own, used only for the structure of the list. Installing a reference on distinct objects, one per thread, ns per objc_setAssociatedObject: 14.4 to 14.6 at 1 thread, 1857.8 to 31.9 at 8, 7313.0 to 53.7 at 24. A striped table of 65536 entries instead reaches 2059.5 at 24 threads. ctest passes 198 of 198.
DTW-Thalion
marked this pull request as ready for review
August 13, 2026 01:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
setReference took the striped lock around a block whose only statement was a test for the preceding search having failed, so replacing the value of a key already present locked around nothing.
The search now decides whether the lock is taken, and a second search runs under it, so two threads no longer install two references for one key.
The lock is no longer the striped one either. lock_for_pointer discards the low 8 bits of the pointer, so two reference lists within 256 bytes take the same lock whatever the size of the table. The list now carries a ThinLock of its own, 4 bytes.
Ns per objc_setAssociatedObject on distinct objects: replacing a value 14.1 to 4.4 at 1 thread and 461.0 to 12.3 at 24; installing a reference 7313.0 to 53.7 at 24. objc_getAssociatedObject is unchanged at 2.7, ctest passes 198 of 198.
Still a draft: the duplicate key race the second search closes has no test, and the unsynchronized key read in findReference wants a memory model opinion.