perf(core): memoize resolveInitialValueForType #7674

ricokahler · 2024-10-25T01:03:53Z

Description

This PR refactors the template permissions resolution to handle templates sequentially instead of in parallel. The key changes are:

Replaces combineLatest with from().pipe(concatMap()) to process templates one at a time
Improves caching of initial value resolution to reduce duplicate computations
Fixes test timeouts by properly mocking async operations and providing complete contexts

These changes were introduced to improve performance and reliability when resolving template permissions, particularly for large numbers of templates. The sequential processing helps prevent potential race conditions and resource contention.

What to review

Key areas to review:

The shift from parallel to sequential processing in templatePermissions.ts
The new caching implementation in resolve.ts including:
- Cache key generation with stableStringify
- Handling of context properties in cache keys
- WeakMap usage for type-level caching
Updated tests that verify both the sequential processing and caching behavior
Mock implementations ensuring proper Observable completion

Testing

The changes are thoroughly tested through:

Updated unit tests for templatePermissions.ts that verify sequential processing
Expanded test suite for resolve.ts covering various caching scenarios:
- Object property order independence
- Array handling
- Null/undefined values
- Context property changes
- Cache invalidation cases
Edge cases for template resolution like missing templates or schema types

Notes for release

Performance improvements for template permissions:

Template permissions are now resolved sequentially instead of in parallel, improving reliability for large numbers of templates
Added smarter caching for initial value resolution, reducing duplicate computations
No API changes or breaking changes - this is an internal optimization

These changes should result in more predictable performance and reduced resource usage when working with templates, particularly in larger projects with many templates.

vercel · 2024-10-25T01:03:58Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
page-building-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:53am
performance-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:53am
test-compiled-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:53am
test-next-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:53am
test-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:53am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
studio-workshop	⬜️ Ignored (Inspect)	Visit Preview		Oct 29, 2024 3:53am

github-actions · 2024-10-25T01:04:48Z

No changes to documentation

github-actions · 2024-10-25T01:10:19Z

Component Testing Report Updated Oct 29, 2024 3:50 AM (UTC)

✅ All Tests Passed -- expand for details

File	Status	Duration	Passed	Skipped
comments/CommentInput.spec.tsx	✅ Passed (Inspect)	46s	15	0
formBuilder/ArrayInput.spec.tsx	✅ Passed (Inspect)	9s	3	0
formBuilder/inputs/PortableText/Annotations.spec.tsx	✅ Passed (Inspect)	32s	6	0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx	✅ Passed (Inspect)	38s	11	7
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx	✅ Passed (Inspect)	0s	0	12
formBuilder/inputs/PortableText/Decorators.spec.tsx	✅ Passed (Inspect)	18s	6	0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx	✅ Passed (Inspect)	10s	3	0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx	✅ Passed (Inspect)	3m 0s	0	0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx	✅ Passed (Inspect)	46s	15	0
formBuilder/inputs/PortableText/Input.spec.tsx	✅ Passed (Inspect)	1m 39s	21	0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx	✅ Passed (Inspect)	1m 17s	18	0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx	✅ Passed (Inspect)	8s	3	9
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx	✅ Passed (Inspect)	26s	9	0
formBuilder/inputs/PortableText/Styles.spec.tsx	✅ Passed (Inspect)	18s	6	0
formBuilder/inputs/PortableText/Toolbar.spec.tsx	✅ Passed (Inspect)	35s	12	0
formBuilder/tree-editing/TreeEditing.spec.tsx	✅ Passed (Inspect)	0s	0	3
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx	✅ Passed (Inspect)	0s	0	3

github-actions · 2024-10-25T01:15:01Z

⚡️ Editor Performance Report

Updated Tue, 29 Oct 2024 03:53:07 GMT

Benchmark	reference ^{latency of sanity@latest}	experiment ^{latency of this branch}	Δ (%) ^{latency difference}
article (title)	17.9 efps (56ms)	17.5 efps (57ms)	+1ms (+1.8%)	✅
article (body)	63.3 efps (16ms)	63.7 efps (16ms)	-0ms (-/-%)	✅
article (string inside object)	17.9 efps (56ms)	17.9 efps (56ms)	+0ms (-/-%)	✅
article (string inside array)	14.4 efps (70ms)	14.7 efps (68ms)	-2ms (-2.2%)	✅
recipe (name)	30.3 efps (33ms)	29.4 efps (34ms)	+1ms (+3.0%)	✅
recipe (description)	34.5 efps (29ms)	32.3 efps (31ms)	+2ms (+6.9%)	✅
recipe (instructions)	99.9+ efps (6ms)	99.9+ efps (7ms)	+1ms (-/-%)	✅
synthetic (title)	14.4 efps (70ms)	14.2 efps (71ms)	+1ms (+1.4%)	✅
synthetic (string inside object)	14.7 efps (68ms)	14.4 efps (70ms)	+2ms (+2.2%)	✅

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark	latency	p75	p90	p99	blocking time	test duration
article (title)	56ms	62ms	70ms	208ms	347ms	12.9s
article (body)	16ms	19ms	22ms	276ms	367ms	5.4s
article (string inside object)	56ms	58ms	65ms	354ms	267ms	8.7s
article (string inside array)	70ms	76ms	83ms	135ms	1079ms	10.2s
recipe (name)	33ms	35ms	39ms	66ms	0ms	9.6s
recipe (description)	29ms	30ms	49ms	115ms	0ms	6.0s
recipe (instructions)	6ms	8ms	9ms	10ms	0ms	3.2s
synthetic (title)	70ms	72ms	83ms	349ms	1367ms	15.3s
synthetic (string inside object)	68ms	72ms	85ms	376ms	1453ms	11.0s

🧪 Experiment result

The performance result of this branch

Benchmark	latency	p75	p90	p99	blocking time	test duration
article (title)	57ms	60ms	69ms	355ms	499ms	13.7s
article (body)	16ms	17ms	21ms	136ms	142ms	5.1s
article (string inside object)	56ms	58ms	64ms	206ms	230ms	8.3s
article (string inside array)	68ms	71ms	79ms	237ms	1106ms	9.7s
recipe (name)	34ms	37ms	43ms	100ms	3ms	9.5s
recipe (description)	31ms	33ms	53ms	83ms	16ms	6.2s
recipe (instructions)	7ms	8ms	9ms	9ms	0ms	3.2s
synthetic (title)	71ms	74ms	86ms	344ms	1435ms	15.9s
synthetic (string inside object)	70ms	75ms	96ms	539ms	1920ms	10.4s

📚 Glossary

column definitions

benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".

latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.

p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.

p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.

p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.

blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.

test duration — how long the test run took to complete.

pedrobonamin

This looks good to me, thanks @ricokahler
Have you tried this updated version in the repo with the issue?

pedrobonamin · 2024-10-29T16:54:42Z

packages/sanity/src/core/store/_legacy/grants/templatePermissions.ts

+      // Collect all permission check observables
+      toArray(),
+      // Switch to combined observable of all permission checks
+      switchMap((observables) => combineLatest(observables)),


So this did the trick! Great

ricokahler · 2024-10-29T16:59:23Z

@pedrobonamin I did and saw a night and day improvement!

ricokahler requested a review from a team as a code owner October 25, 2024 01:03

ricokahler requested review from binoy14 and removed request for a team October 25, 2024 01:03

vercel bot deployed to Preview – test-next-studio October 25, 2024 01:06 View deployment

vercel bot deployed to Preview – test-compiled-studio October 25, 2024 01:09 View deployment

ricokahler requested review from cngonzalez and pedrobonamin and removed request for binoy14 October 25, 2024 01:12

ricokahler force-pushed the sdk-7 branch from f21daec to 9805053 Compare October 29, 2024 03:01

vercel bot deployed to Preview – performance-studio October 29, 2024 03:05 View deployment

vercel bot deployed to Preview – page-building-studio October 29, 2024 03:05 View deployment

vercel bot deployed to Preview – test-studio October 29, 2024 03:05 View deployment

vercel bot deployed to Preview – test-next-studio October 29, 2024 03:10 View deployment

vercel bot deployed to Preview – test-compiled-studio October 29, 2024 03:13 View deployment

ricokahler added 2 commits October 28, 2024 22:33

fix(core): resolve template permissions sequentially

a8a5d1e

perf(core): memoize resolveInitialValueForType

d5606e6

ricokahler force-pushed the sdk-7 branch from 9805053 to d5606e6 Compare October 29, 2024 03:42

vercel bot deployed to Preview – page-building-studio October 29, 2024 03:45 View deployment

vercel bot deployed to Preview – performance-studio October 29, 2024 03:45 View deployment

vercel bot deployed to Preview – test-studio October 29, 2024 03:46 View deployment

vercel bot deployed to Preview – test-next-studio October 29, 2024 03:50 View deployment

vercel bot deployed to Preview – test-compiled-studio October 29, 2024 03:53 View deployment

pedrobonamin approved these changes Oct 29, 2024

View reviewed changes

ricokahler added this pull request to the merge queue Oct 29, 2024

Merged via the queue into next with commit 3602d67 Oct 29, 2024
67 checks passed

ricokahler deleted the sdk-7 branch October 29, 2024 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(core): memoize resolveInitialValueForType #7674

perf(core): memoize resolveInitialValueForType #7674

ricokahler commented Oct 25, 2024

vercel bot commented Oct 25, 2024 •

edited

Loading

github-actions bot commented Oct 25, 2024

github-actions bot commented Oct 25, 2024 •

edited

Loading

github-actions bot commented Oct 25, 2024 •

edited

Loading

🏠 Reference result

🧪 Experiment result

📚 Glossary

column definitions

pedrobonamin left a comment

pedrobonamin Oct 29, 2024

ricokahler commented Oct 29, 2024

perf(core): memoize resolveInitialValueForType #7674

perf(core): memoize resolveInitialValueForType #7674

Conversation

ricokahler commented Oct 25, 2024

Description

What to review

Testing

Notes for release

vercel bot commented Oct 25, 2024 • edited Loading

github-actions bot commented Oct 25, 2024

github-actions bot commented Oct 25, 2024 • edited Loading

github-actions bot commented Oct 25, 2024 • edited Loading

⚡️ Editor Performance Report

🏠 Reference result

🧪 Experiment result

📚 Glossary

column definitions

pedrobonamin left a comment

Choose a reason for hiding this comment

pedrobonamin Oct 29, 2024

Choose a reason for hiding this comment

ricokahler commented Oct 29, 2024

vercel bot commented Oct 25, 2024 •

edited

Loading

github-actions bot commented Oct 25, 2024 •

edited

Loading

github-actions bot commented Oct 25, 2024 •

edited

Loading