Fold optional TextAttributes fields into a presence mask when hashing (#57984) - #57984
Fold optional TextAttributes fields into a presence mask when hashing (#57984)#57984Abbondanzo wants to merge 1 commit into
Conversation
|
@Abbondanzo has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115621401. |
…react#57984) Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Differential Revision: D115621401
f6f3445 to
cc2d909
Compare
…react#57984) Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Differential Revision: D115621401
b9d26fa to
25f10d6
Compare
…react#57984) Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Reviewed By: javache Differential Revision: D115621401
25f10d6 to
533ba3e
Compare
…react#57984) Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Reviewed By: javache Differential Revision: D115621401
|
/review |
🤖 AI code reviewDecision: Ready for human review Overall PR risk: Low. Change modifies existing behavior in TextAttributes hashing and equality used by the AttributedString display cache and TextMeasureCache. Blast radius is limited to the text measurement/layout path with no persistence or public API surface and straightforward rollback to prior hash ordering. No defect findings remained after consolidation; the most plausible effect if shipped is in-memory cache invalidation to misses with no semantic break. No findings. This review is advisory — it never blocks a merge and never auto-approves. |
…react#57984) Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Reviewed By: javache Differential Revision: D115621401
533ba3e to
5f1fe0c
Compare
|
This pull request has been merged in 8cf8e09. |
Summary:
hash_combinemixes each field into the previous seed, so it forms a dependency chain the CPUcannot overlap and an unset optional still costs a full link.
hash_combine_optionalsfolds a runof optionals into one presence-mask link plus the engaged values, so a further optional costs a bit
in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged
fields alone would make the same value in two different slots hash identically.
Equality moves from
std::tieto a short-circuit chain ordered cheapest first, with the string andvector fields last, because the dominant caller is a successful cache lookup where the keys are
equal and every field has to be examined.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D115621401