perf(zero-solid): use reconcile to minimize reactive changes on query changes #4575
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Bencher Report| Branch | grgbkr/solid-id |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | File Size | Benchmark Result kilobytes (KB) (Result Δ%) | Upper Boundary kilobytes (KB) (Limit %) |
|---|---|---|---|
| zero-package.tgz | 📈 view plot 🚷 view threshold | 1,211.68 KB(+0.07%)Baseline: 1,210.86 KB | 1,235.07 KB (98.11%) |
| zero.js | 📈 view plot 🚷 view threshold | 199.88 KB(+0.05%)Baseline: 199.77 KB | 203.77 KB (98.09%) |
| zero.js.br | 📈 view plot 🚷 view threshold | 56.02 KB(+0.04%)Baseline: 56.00 KB | 57.12 KB (98.07%) |
|
| Branch | grgbkr/solid-id |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | Throughput | Benchmark Result operations / second (ops/s) x 1e3 (Result Δ%) | Lower Boundary operations / second (ops/s) x 1e3 (Limit %) |
|---|---|---|---|
| src/client/custom.bench.ts > big schema | 📈 view plot 🚷 view threshold | 395.66 ops/s x 1e3(+10.26%)Baseline: 358.85 ops/s x 1e3 | 328.54 ops/s x 1e3 (83.04%) |
| src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 1.63 ops/s x 1e3(+2.35%)Baseline: 1.59 ops/s x 1e3 | 1.54 ops/s x 1e3 (94.73%) |
| src/client/zero.bench.ts > pk compare > pk = N | 📈 view plot 🚷 view threshold | 30.43 ops/s x 1e3(+0.18%)Baseline: 30.38 ops/s x 1e3 | 29.47 ops/s x 1e3 (96.85%) |
| src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 2.61 ops/s x 1e3(+3.62%)Baseline: 2.52 ops/s x 1e3 | 2.45 ops/s x 1e3 (93.71%) |
| import type {Entry, Format} from './view.ts'; | ||
|
|
||
| export const refCountSymbol = Symbol('rc'); | ||
| export const idSymbol = Symbol('id'); |
There was a problem hiding this comment.
Or should this be
export const keySymbol = Symbol('key');
There was a problem hiding this comment.
I don't really care. This is fine
| expect(resultDetailsLog).toEqual([{type: 'complete'}]); | ||
| }); | ||
|
|
||
| test('useQuery query deps change, reconcile minimizes reactive updates', async () => { |
There was a problem hiding this comment.
This and the next test cases are new and verify this is giving us the precise reactive updates we want.
| expect(resultDetailsLog).toEqual([{type: 'complete'}]); | ||
| resetLogs(); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises |
There was a problem hiding this comment.
I don't know why the linter is getting this wrong... this method does not return a promise.
|
FYI @devagrawal09 |
| rc: number, | ||
| ): RCEntry { | ||
| const id = withIDs ? makeID(row, schema) : ''; | ||
| return {...row, [refCountSymbol]: rc, [idSymbol]: id}; |
There was a problem hiding this comment.
@arv I know you were concerned with polymorphic object shape. Does this look ok? Would it be ok to make the [idSymbol] optional?
There was a problem hiding this comment.
I think we do not need to add the symbol if withIDs is false. In non SolidJS it will always be false and in SolidJS it will always be true.
| import type {Entry, Format} from './view.ts'; | ||
|
|
||
| export const refCountSymbol = Symbol('rc'); | ||
| export const idSymbol = Symbol('id'); |
There was a problem hiding this comment.
I don't really care. This is fine
| rc: number, | ||
| ): RCEntry { | ||
| const id = withIDs ? makeID(row, schema) : ''; | ||
| return {...row, [refCountSymbol]: rc, [idSymbol]: id}; |
There was a problem hiding this comment.
I think we do not need to add the symbol if withIDs is false. In non SolidJS it will always be false and in SolidJS it will always be true.
Great, I will send a change to clean this up. |
This is a follow up to: #4559
To effectively use solidjs's
reconcileutility it was necessary to add a consistent identity key to every row in the view.