diff --git a/.agents/docs/binding-mapping.md b/.agents/docs/binding-mapping.md index a66c82a..a528497 100644 --- a/.agents/docs/binding-mapping.md +++ b/.agents/docs/binding-mapping.md @@ -92,12 +92,14 @@ Arbitrary context stays in a JavaScript map keyed by current public NodeId. Nati `setNodeContext` updates native presence and the JavaScript map and marks the node dirty. In-place context changes and callback-captured data cannot be observed automatically; callers use `markDirty` when those changes affect later measurement. Supplying a different callback does not invalidate Taffy's cache by itself. -The measure callback runs synchronously and receives owned `knownDimensions`, `availableSpace`, public NodeId, the original JavaScript context, and a detached Style snapshot. Taffy controls whether, when, and how often it runs. It must return a complete `{ width, height }` number record; Promises, missing axes, and invalid values throw `TypeError`. +The measure callback runs synchronously and receives owned `knownDimensions`, `availableSpace`, public NodeId, the original JavaScript context, and a detached Style snapshot. Taffy controls the requested nodes, constraints, and ordering, subject to the exact-repeat reuse below. The callback must return a complete `{ width, height }` number record; Promises, missing axes, and invalid values throw `TypeError`. The native owner uses checked `RefCell` access. A native-backed call on the same tree during measurement throws `ERR_TAFFY_TREE_BUSY`; JavaScript-only value operations and another tree remain usable. On the first callback throw or invalid result, the bridge retains that failure, stops further JavaScript callbacks, lets Taffy's infallible stack finish with internal zero sizes, invalidates the requested subtree, and throws synchronously. A thrown JavaScript value keeps its identity. The tree remains usable, but already completed JavaScript side effects and stored Layout work are not rolled back. +Each `computeLayoutWithMeasure` creates one Rust `MeasureSession` that reuses a successful result when Taffy repeats the exact same request during that compute. The key contains the raw NodeId, both optional known dimensions, and both available-space variants and definite values; every `f32` uses its exact bit representation. Cache lookup happens before creating callback arguments or converting Style, and callback throws or binding conversion failures are never stored. The session and its cache are dropped when the compute returns, so caller-managed dirtying and Taffy's persistent cache semantics remain unchanged across computes. + ## Mutation, errors, and panic containment Validate complete input, every involved NodeId, topology, index, and range before the first ordinary mutation. Failed single-value and collection mutations must not leave partial wrapper or native state. Measured computation is the documented exception because callback failure happens after computation has started. diff --git a/.agents/docs/taffyjs-node-decisions.md b/.agents/docs/taffyjs-node-decisions.md index f4c5bdc..639bf2a 100644 --- a/.agents/docs/taffyjs-node-decisions.md +++ b/.agents/docs/taffyjs-node-decisions.md @@ -56,6 +56,16 @@ The callback returns a complete numeric size. The first thrown value or invalid Retained, asynchronous, off-thread, cancellable, or transactionally rolled-back measurement would be a different API with its own ownership and failure contract. +## Exact measure request reuse within one compute + +**Ruling:** During one `computeLayoutWithMeasure` call, the Rust binding must invoke the public JavaScript measure callback at most once for an exact combination of raw NodeId, both optional known dimensions, and both available-space variants and definite values. Floating-point equality must use the exact `f32` bit representation. Only a callback result that successfully converts to `Size` may be reused. + +**Limits:** Reuse ends with the current compute and adds no persistent tree, Style, context, or JavaScript cache. Style and context are not key inputs because the public tree API cannot mutate either for the same node while its measure callback is running. This does not add a per-node measure API, reuse Style snapshots, or change Taffy's layout or measurement phases. The existing first-failure behavior, subtree invalidation, thrown-value identity, and later retry behavior remain unchanged. + +**Why:** The coding-agent chat initial-layout workload showed that Taffy issued 3,420 measure requests but only 1,262 exact argument combinations. Reusing those successful results before Style conversion and the Node-API or WASI boundary removes repeated boundary work without changing a caller-visible input, broadening cache invalidation, or retaining results after the synchronous operation. + +**Source:** Yunfei (`@hyfdev`), 2026-08-17; specified the exact key, lifetime, success-only insertion, failure semantics, non-goals, Native/WASI coverage, and benchmark acceptance criteria in [issue #38](https://github.com/hyfdev/taffyjs/issues/38). + ## Selective query ### Complete bounded per-node selective reads diff --git a/crates/taffyjs_binding/src/measure.rs b/crates/taffyjs_binding/src/measure.rs index 269dbc6..303809a 100644 --- a/crates/taffyjs_binding/src/measure.rs +++ b/crates/taffyjs_binding/src/measure.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::marker::PhantomData; use std::ptr; use std::rc::Rc; @@ -38,6 +39,48 @@ pub struct MeasureResultInput { pub height: f64, } +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +enum AvailableSpaceCacheKey { + Definite(u32), + MinContent, + MaxContent, +} + +impl From for AvailableSpaceCacheKey { + fn from(value: AvailableSpace) -> Self { + match value { + AvailableSpace::Definite(value) => Self::Definite(value.to_bits()), + AvailableSpace::MinContent => Self::MinContent, + AvailableSpace::MaxContent => Self::MaxContent, + } + } +} + +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +struct MeasureCacheKey { + node: u64, + known_width: Option, + known_height: Option, + available_width: AvailableSpaceCacheKey, + available_height: AvailableSpaceCacheKey, +} + +impl MeasureCacheKey { + fn new( + node: NodeId, + known_dimensions: Size>, + available_space: Size, + ) -> Self { + Self { + node: u64::from(node), + known_width: known_dimensions.width.map(f32::to_bits), + known_height: known_dimensions.height.map(f32::to_bits), + available_width: available_space.width.into(), + available_height: available_space.height.into(), + } + } +} + pub(crate) enum MeasureFailure<'env> { Callback(Unknown<'env>), Binding(BindingError), @@ -45,6 +88,7 @@ pub(crate) enum MeasureFailure<'env> { pub(crate) struct MeasureSession<'env> { callback: Function<'env, MeasureArguments, Unknown<'env>>, + cache: HashMap>, failure: Option>, not_send: PhantomData>, } @@ -53,6 +97,7 @@ impl<'env> MeasureSession<'env> { pub(crate) fn new(callback: Function<'env, MeasureArguments, Unknown<'env>>) -> Self { Self { callback, + cache: HashMap::new(), failure: None, not_send: PhantomData, } @@ -69,13 +114,21 @@ impl<'env> MeasureSession<'env> { return Size::ZERO; } + let cache_key = MeasureCacheKey::new(node, known_dimensions, available_space); + if let Some(size) = self.cache.get(&cache_key) { + return *size; + } + let result = call( &self.callback, self.arguments(known_dimensions, available_space, node, style), ) .and_then(|value| result_size(value).map_err(MeasureFailure::Binding)); match result { - Ok(size) => size, + Ok(size) => { + self.cache.insert(cache_key, size); + size + } Err(failure) => { self.failure = Some(failure); Size::ZERO @@ -190,10 +243,69 @@ pub(crate) fn invalidate_subtree(tree: &mut TaffyTree<()>, root: NodeId) -> Bind mod tests { use std::marker::PhantomData; - use taffy::TaffyTree; - use taffy::style::Style; + use taffy::geometry::Size; + use taffy::style::{AvailableSpace, Style}; + use taffy::{NodeId, TaffyTree}; + + use super::{AvailableSpaceCacheKey, MeasureCacheKey, MeasureSession, invalidate_subtree}; - use super::{MeasureSession, invalidate_subtree}; + fn cache_key( + node: u64, + known_width: Option, + known_height: Option, + available_width: AvailableSpace, + available_height: AvailableSpace, + ) -> MeasureCacheKey { + MeasureCacheKey::new( + NodeId::from(node), + Size { + width: known_width, + height: known_height, + }, + Size { + width: available_width, + height: available_height, + }, + ) + } + + #[test] + fn measure_cache_key_preserves_every_exact_input() { + let first_nan = f32::from_bits(0x7fc0_0000); + let second_nan = f32::from_bits(0x7fc0_0001); + assert_eq!( + cache_key( + 7, + Some(-0.0), + Some(first_nan), + AvailableSpace::Definite(-0.0), + AvailableSpace::Definite(second_nan), + ), + MeasureCacheKey { + node: 7, + known_width: Some((-0.0f32).to_bits()), + known_height: Some(first_nan.to_bits()), + available_width: AvailableSpaceCacheKey::Definite((-0.0f32).to_bits()), + available_height: AvailableSpaceCacheKey::Definite(second_nan.to_bits()), + } + ); + assert_eq!( + cache_key( + 8, + None, + None, + AvailableSpace::MinContent, + AvailableSpace::MaxContent, + ), + MeasureCacheKey { + node: 8, + known_width: None, + known_height: None, + available_width: AvailableSpaceCacheKey::MinContent, + available_height: AvailableSpaceCacheKey::MaxContent, + } + ); + } #[test] fn measure_session_stays_on_the_javascript_thread() { diff --git a/tests/taffyjs-node/tests/tree/compute-layout-with-measure.test.mts b/tests/taffyjs-node/tests/tree/compute-layout-with-measure.test.mts index e8611ee..37f7201 100644 --- a/tests/taffyjs-node/tests/tree/compute-layout-with-measure.test.mts +++ b/tests/taffyjs-node/tests/tree/compute-layout-with-measure.test.mts @@ -5,6 +5,8 @@ import { AvailableSpace, AvailableSpaceKind, Dimension, + Display, + FlexDirection, GridPlacement, GridPlacementKind, LengthUnit, @@ -39,6 +41,127 @@ function compute(tree: TaffyTree, root: NodeId, measure: MeasureFunction["availableSpace"]["width"]): string { + if (value.kind === AvailableSpaceKind.Definite) { + return `definite:${f32Bits(value.value)}`; + } + return value.kind === AvailableSpaceKind.MinContent ? "min-content" : "max-content"; +} + +function measureRequestKey(args: MeasureArgs): MeasureRequestKey { + return [ + String(args.node), + knownDimensionKey(args.knownDimensions.width), + knownDimensionKey(args.knownDimensions.height), + availableSpaceKey(args.availableSpace.width), + availableSpaceKey(args.availableSpace.height), + ]; +} + +function hasPairDifferingOnlyAt( + requests: readonly MeasureRequestKey[], + component: number, + acceptsDifference: (left: string, right: string) => boolean = () => true, +): boolean { + for (const [leftIndex, left] of requests.entries()) { + for (const right of requests.slice(leftIndex + 1)) { + if ( + left[component] !== right[component] && + acceptsDifference(left[component], right[component]) && + left.every((value, index) => index === component || value === right[index]) + ) { + return true; + } + } + } + return false; +} + +function assertNoDuplicateMeasureRequests(requests: readonly MeasureRequestKey[]): void { + const uniqueRequests = new Set(requests.map((request) => JSON.stringify(request))); + assert.equal( + uniqueRequests.size, + requests.length, + "one compute must enter JavaScript once for each exact measure request", + ); +} + +function createNestedMeasureFixture( + firstDirection: typeof FlexDirection.Row | typeof FlexDirection.Column, +): NestedMeasureFixture { + const tree = new TaffyTree(); + tree.disableRounding(); + const measured = tree.newLeafWithContext( + { flexShrink: 1, minSize: { width: 0, height: 0 } }, + true, + ); + let nested = measured; + for (let depth = 0; depth < 4; depth += 1) { + const otherDirection = + firstDirection === FlexDirection.Row ? FlexDirection.Column : FlexDirection.Row; + const flexDirection = depth % 2 === 0 ? firstDirection : otherDirection; + nested = tree.newWithChildren( + { + display: Display.Flex, + flexDirection, + flexGrow: depth === 3 ? 1 : 0, + flexShrink: 1, + minSize: { width: 0, height: 0 }, + padding: 3, + gap: 2, + }, + [nested], + ); + } + const fixed = tree.newLeaf({ size: { width: 264, height: 100 } }); + const root = tree.newWithChildren( + { + display: Display.Flex, + flexDirection: FlexDirection.Row, + size: { width: 1280, height: 800 }, + padding: 16, + }, + [fixed, nested], + ); + return { tree, measured, root }; +} + +function collectMeasureRequests(fixture: NestedMeasureFixture): MeasureRequestKey[] { + const requests: MeasureRequestKey[] = []; + fixture.tree.computeLayoutWithMeasure({ + root: fixture.root, + availableSpace: { width: 1280, height: 800 }, + measure(args) { + requests.push(measureRequestKey(args)); + return { width: 73, height: 19 }; + }, + }); + return requests; +} + test("callback-args", () => { const tree = new TaffyTree(); const context = { label: "callback" }; @@ -101,6 +224,105 @@ test("result-f32", () => { assert.deepEqual(nanTree.getUnroundedLayout(nan).size, { width: 0, height: 0 }); }); +test("identical requests reuse one callback result without merging constraints", () => { + const rowFirst = collectMeasureRequests(createNestedMeasureFixture(FlexDirection.Row)); + const columnFirst = collectMeasureRequests(createNestedMeasureFixture(FlexDirection.Column)); + assertNoDuplicateMeasureRequests(rowFirst); + assertNoDuplicateMeasureRequests(columnFirst); + + assert.equal( + hasPairDifferingOnlyAt(rowFirst, 1), + true, + "known width remains part of the request key", + ); + assert.equal( + hasPairDifferingOnlyAt(columnFirst, 2), + true, + "known height remains part of the request key", + ); + + const kind = (value: string) => (value.startsWith("definite:") ? "definite" : value); + const kindsDiffer = (left: string, right: string) => kind(left) !== kind(right); + const definiteValuesDiffer = (left: string, right: string) => + left.startsWith("definite:") && right.startsWith("definite:"); + assert.equal( + hasPairDifferingOnlyAt(columnFirst, 3, kindsDiffer), + true, + "available width kind remains part of the request key", + ); + assert.equal( + hasPairDifferingOnlyAt(columnFirst, 3, definiteValuesDiffer), + true, + "definite available width remains part of the request key", + ); + assert.equal( + hasPairDifferingOnlyAt(rowFirst, 4, kindsDiffer), + true, + "available height kind remains part of the request key", + ); + assert.equal( + hasPairDifferingOnlyAt(rowFirst, 4, definiteValuesDiffer), + true, + "definite available height remains part of the request key", + ); +}); + +test("identical constraints on different nodes remain separate requests", () => { + const tree = new TaffyTree(); + const first = tree.newLeafWithContext( + { flexGrow: 1, flexShrink: 1, minSize: { width: 0 } }, + "first", + ); + const second = tree.newLeafWithContext( + { flexGrow: 1, flexShrink: 1, minSize: { width: 0 } }, + "second", + ); + const root = tree.newWithChildren( + { + display: Display.Flex, + flexDirection: FlexDirection.Row, + size: { width: 200, height: 100 }, + }, + [first, second], + ); + const requests: MeasureRequestKey[] = []; + const measuredNodes = new Set(); + + tree.computeLayoutWithMeasure({ + root, + availableSpace: { width: 200, height: 100 }, + measure(args) { + requests.push(measureRequestKey(args)); + measuredNodes.add(args.node); + return { width: 30, height: 10 }; + }, + }); + + assert.equal(measuredNodes.has(first), true); + assert.equal(measuredNodes.has(second), true); + assert.equal( + hasPairDifferingOnlyAt(requests, 0), + true, + "node identity remains part of the request key", + ); +}); + +test("measure request reuse ends when compute returns", () => { + const fixture = createNestedMeasureFixture(FlexDirection.Row); + const first = collectMeasureRequests(fixture); + assertNoDuplicateMeasureRequests(first); + + fixture.tree.markDirty(fixture.measured); + const second = collectMeasureRequests(fixture); + assertNoDuplicateMeasureRequests(second); + const firstRequests = new Set(first.map((request) => JSON.stringify(request))); + assert.equal( + second.some((request) => firstRequests.has(JSON.stringify(request))), + true, + "a later compute must re-enter JavaScript for a repeated Taffy request", + ); +}); + test("cache-calls", () => { const tree = new TaffyTree(); const node = tree.newLeafWithContext({}, true); @@ -215,12 +437,23 @@ test("throw-identity", () => { for (const thrown of [{ reason: "stop" }, "stop", 17, null]) { const tree = new TaffyTree(); const node = tree.newLeafWithContext({}, true); + let failedCalls = 0; const received = captureError(() => compute(tree, node, () => { + failedCalls += 1; throw thrown; }), ); assert.equal(received, thrown); + assert.equal(failedCalls, 1); + + let retryCalls = 0; + compute(tree, node, () => { + retryCalls += 1; + return { width: 30, height: 10 }; + }); + assert.equal(retryCalls > 0, true, "a thrown callback result must not be cached"); + assert.deepEqual(tree.getUnroundedLayout(node).size, { width: 30, height: 10 }); } });