Skip to content

dia.Cell: prop()/attr() deep-clones and deep-merges all attributes on every path write #3513

Description

@kumilingus

Summary

Cell.prop(path, value) (and therefore Cell.attr(path, value)) deep-clones all attributes of the cell and deep-merges the update into the clone on every call. The cost of writing one attribute scales with the size of the whole attrs tree (and ports, markup when stored on the model, ...), not with the size of the change. Replace the full deep merge with a set-by-path that clones only the spine of the path (structural sharing).

Spun off from #3512, where the model side turned out to dominate the cost of simple attribute changes: with #3511 applied, cell.attr('body/stroke', v) on 1000 standard.Rectangle elements spends ~5 ms in the view update and ~17 ms before the view is even reached.

Current behaviour

packages/joint-core/src/dia/Cell.mjs, prop() with a path:

var baseAttributes = merge({}, this.attributes);              // deep clone of every attribute of the cell
options.rewrite && unsetByPath(baseAttributes, path, '/');
var attributes = merge(baseAttributes, update, config.cellMergeStrategy);   // deep merge of the update
return this.set(property, attributes[property], options);

Then Model.set() runs isEqual(current[attr], val) on the result, which is another deep walk.

The object form (prop({ attrs: {...} })) does the same per top-level key (merge(merge({}, { changedValue: current }), { changedValue: next })).

Measurement

Node 22, no paper attached, 1000 standard.Rectangle, median of 5 rounds, one write per cell per round. "extra selectors" adds N more selectors with 4 attributes each to every cell's attrs, to simulate richer shapes.

extra selectors attr('body/stroke', v) attr({ body: { stroke } }) merge({}, attributes) alone set('attrs', spineClone) position(x, y)
0 7.9 ms 5.7 ms 1.8 ms 2.6 ms 2.1 ms
10 10.4 ms 9.8 ms 5.2 ms 2.9 ms 1.8 ms
30 19.5 ms 18.3 ms 12.1 ms 4.5 ms 1.8 ms

set('attrs', spineClone) is the manual equivalent of structural sharing:

const a = cell.get('attrs');
cell.set('attrs', { ...a, body: { ...a.body, stroke: v } });

It is 3–4× cheaper than attr() today and stays almost flat as attrs grows, while attr() grows linearly with the tree size.

Benchmark script
import { dia, shapes, util } from '@joint/core';
const N = 1000, ROUNDS = 5;
const median = a => [...a].sort((x, y) => x - y)[Math.floor(a.length / 2)];
function build(extraKeys) {
    const graph = new dia.Graph({}, { cellNamespace: shapes });
    const cells = [];
    for (let i = 0; i < N; i++) {
        const el = new shapes.standard.Rectangle({ position: { x: i, y: i }, size: { width: 80, height: 40 }, attrs: { label: { text: 'r' + i } } });
        for (let k = 0; k < extraKeys; k++) el.attr('extra' + k, { fill: '#000', stroke: '#111', strokeWidth: 2, rx: 3 }, { silent: true });
        cells.push(el);
    }
    graph.resetCells(cells);
    return cells;
}
function time(fn) { const r = []; for (let i = 0; i < ROUNDS; i++) { const t = performance.now(); fn(i); r.push(performance.now() - t); } return +median(r).toFixed(1); }
for (const extra of [0, 10, 30]) {
    const cells = build(extra);
    console.log({
        extra,
        'attr(path)': time(r => { for (const c of cells) c.attr('body/stroke', r % 2 ? '#f00' : '#00f'); }),
        'attr(object)': time(r => { for (const c of cells) c.attr({ body: { stroke: r % 2 ? '#f00' : '#00f' } }); }),
        'merge only': time(() => { for (const c of cells) util.merge({}, c.attributes); }),
        'set spine clone': time(r => { for (const c of cells) { const a = c.get('attrs'); c.set('attrs', { ...a, body: { ...a.body, stroke: r % 2 ? '#f00' : '#00f' } }); } }),
        'position': time(r => { for (const c of cells) c.position(r, r); }),
    });
}

Proposal

  1. In the path form of prop(), clone only the top-level property being written and the objects along the path (attrs → attrs.body), reuse every other subtree by reference, and set the leaf. No merge({}, this.attributes), no deep merge of the update.
  2. Keep the semantics that matter:
    • rewrite: true replaces the value at the path instead of merging into it (currently unsetByPath + merge). With spine cloning this is just "assign the leaf" vs "merge into the leaf object" (leaf-level merge only, using config.cellMergeStrategy).
    • Array indices in the path create arrays (vertices/0/x), integer-key limitation stays as documented.
    • options.propertyPath / propertyValue / propertyPathArray are still set on the options for listeners (Inspector relies on them).
    • change:attrs payload must remain a new object so previous('attrs') differs by reference; structural sharing preserves that for the spine and, as a bonus, makes isEqual() in Model.set() short-circuit on untouched subtrees (reference equality at each level).
  3. The object form (prop({ attrs: {...} })) can use the same helper per top-level key instead of the double merge.
  4. Tests to add: subtrees not on the written path keep their reference identity after attr() (this is the property that makes it cheap and it should be locked in); rewrite behaviour unchanged; array paths unchanged; cellMergeStrategy still applied for leaf objects.

Risks

  • Any code that mutates cell.get('attrs') in place after reading it would now also mutate the previous state (previous('attrs') shares subtrees). That is already unsupported, but worth a note in the changelog.
  • config.cellMergeStrategy today applies to the whole tree; after the change it only applies where the update and the current value are both objects at the leaf. Confirm no shape relies on array-merge semantics above the leaf.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions