Skip to content

Lazily compute whether attribute changed in _attributeModified(_:old:new:) #1237

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

Conversation

jmschonfeld
Copy link
Contributor

_attributeModified is called when an attribute value in an attribute storage dictionary was replaced by a new value. It is responsible for enforcing the invalidation constraints on attributes by removing any dependent attributes from the storage (recursively). Currently (assuming both a new and old value exist) this function unconditionally compares the new and old values to return early if the value didn't change (meaning attributes shouldn't be invalidated). However, this requires unboxing the existential attribute values and calling through to their == implementation which can be expensive. Instead, we can make this calculation lazy, only performing it when we actually find another attribute dependent upon the changed one. In cases where there are no dependent attributes, we avoid the equality check altogether as it is unnecessary. This shows about a 6% to 11% improvement in the setAttribute benchmark:

╒══════════════════════════════════════════╤═══════════╤═══════════╤═══════════╤═══════════╤═══════════╤═══════════╤═══════════╤═══════════╕
│          Throughput (# / s) (K)          │        p0 │       p25 │       p50 │       p75 │       p90 │       p99 │      p100 │   Samples │
╞══════════════════════════════════════════╪═══════════╪═══════════╪═══════════╪═══════════╪═══════════╪═══════════╪═══════════╪═══════════╡
│                   main                   │       141 │       140 │       139 │       137 │       135 │       125 │       122 │       138 │
├──────────────────────────────────────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┤
│                  branch                  │       150 │       148 │       147 │       146 │       145 │       142 │       136 │       147 │
├──────────────────────────────────────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┤
│                    Δ                     │         9 │         8 │         8 │         9 │        10 │        17 │        14 │         9 │
├──────────────────────────────────────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┼───────────┤
│              Improvement %               │         6 │         6 │         6 │         7 │         7 │        14 │        11 │         9 │
╘══════════════════════════════════════════╧═══════════╧═══════════╧═══════════╧═══════════╧═══════════╧═══════════╧═══════════╧═══════════╛

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

@jmschonfeld jmschonfeld merged commit 201a45b into swiftlang:main Apr 7, 2025
3 checks passed
@jmschonfeld jmschonfeld deleted the attrstr/attribute-modified-lazy-diff branch April 7, 2025 22:07
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