Skip to content

perf(ids): fetch the applicable items once per specification run - #866

Open
havs-mc wants to merge 2 commits into
ThatOpen:mainfrom
havs-mc:perf/ids-share-items-data
Open

havs-mc wants to merge 2 commits into
ThatOpen:mainfrom
havs-mc:perf/ids-share-items-data

Conversation

@havs-mc

@havs-mc havs-mc commented Nov 11, 2026

Copy link
Copy Markdown

Description

Each attribute, entity and property requirement of a specification calls model.getItemsData for the same applicable items. Each call has a large fixed cost in the fragments worker. On a 15 MB model, one call took ~4 s, and 10 calls of 200 items took 45 s against 7.5 s for one call of 2 000. A specification with many property requirements spends nearly all its time re-fetching.

  • IDSSpecification.test() opens a shared scope for its applicable items. Requirements fetch each kind of item data once (attributes; property-set relations) and share it. Outside a specification run, facets fetch as before.
  • IDSProperty.getPsets() no longer modifies the fetched occurrence sets when adding type properties. It works on a copy, since the data is now shared.
  • IDSProperty.test()'s fetch no longer follows an element's type back to its other instances. In getItemsData, a relation without its own entry takes the config of the relation it was reached through. So ObjectTypeOf under the type inherited IsDefinedBy's relations: true and returned every instance of the type with all their property sets. It is now listed as { attributes: false, relations: false }, as DefinesOccurrence already was. On a 9 600-object model with shared types, one such fetch ran for over 8 minutes without finishing.

Additional context

  • On a real 15 MB structural model, a specification with ~30 property requirements went from ~100 s to ~8 s with the equivalent local patch.
  • The buildingSMART implementer test cases give the same result as on main (231 of 307 agree).
  • New specification-items-data.test.ts: five requirements make two full fetches instead of five, and the results equal those of each requirement run on its own. No fetched item data contains ObjectTypeOf; that test fails without the config change.
  • New API surface: IDSFacet.startSharedItemsData / endSharedItemsData (static) and the protected getSharedItemsData. The cache is a module-level WeakMap keyed on the applicable-items object of one run, so separate Components instances don't share anything. Happy to make these internal if you prefer another shape, e.g. passing a cache in the config argument of test().
  • Split from the fixes discussed in IDS conformance: status against buildingSMART's implementer test cases, and what is left #831, as suggested there.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other (performance)

Before submitting the PR, please make sure you do the following:

  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Follow the Conventional Commits v1.0.0 standard for PR naming.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves.
  • Ideally, include relevant tests that fail without this PR but pass with it.

🤖 Generated with Claude Code

havs-mc and others added 2 commits November 11, 2026 15:10
Every attribute, entity and property requirement fetched the same items with
getItemsData, and each call has a large fixed cost in the fragments worker.
IDSSpecification.test now lets its requirements share one fetch per kind of
item data. IDSProperty no longer adds type properties into the fetched
occurrence sets, since that data is now shared.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erty

In fragments' getItemsData a relation without its own entry in `relations`
takes the config of the relation it was reached through. The type reached
through IsDefinedBy ({ relations: true }) therefore followed ObjectTypeOf to
every other instance of that type, with all their property sets. On models
where many elements share types, one fetch could pull in most of the model.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant