Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unbounded cache sizes #83

Open
MichaelBelousov opened this issue Jun 26, 2023 · 3 comments
Open

Remove unbounded cache sizes #83

MichaelBelousov opened this issue Jun 26, 2023 · 3 comments
Assignees
Labels
performance potential performance increase

Comments

@MichaelBelousov
Copy link
Contributor

MichaelBelousov commented Jun 26, 2023

Unbounded caches in use are problematic:

  • js Map in v8 (node) has a limit of 16.7M elements which has already been surpassed by Aspect counts in iModels
  • large memory usage and garbage collector performance impacted by too many objects
  • source element changes cache in federation-guid-optimization branch

Proposed changes:

  • element remap table should be a temporary sqlite db, not a C++ data structure, so it can page to disk under memory pressure, and still be used in native code where geometry part remapping is done.
  • aspect and element remap tables are probably, as the transformer uses them, good candidates for a custom data structure that stores "runs" (e.g. run-length encoding) of element mappings, (e.g. 7-10+10,15->16 === 7->17,8->18,9->19->10->20,15->16).
  • the entire pending reference cache can be removed by storing a smaller cache of just the ids of elements that had unresolved references, and then after exporting everything else which will have inserted all targeted elements, then do the second transform + update that currently occurs as soon as the references are found.

Short term solutions:

  • Use BigMap classes for aspects.
@MichaelBelousov
Copy link
Contributor Author

worth noting, last time I tried using the integer based map in iTwin.js core for the transformer, it worsened performance 5-10% iirc. We need an integer based map though to do a naive run-compressed map. So need to think about this.

@MichaelBelousov
Copy link
Contributor Author

MichaelBelousov commented Aug 14, 2023

this is done partially in the early-polymorphic-insert branch

EDIT: that branch now includes a CompactRemapTable class which does this remapping entirely

@MichaelBelousov MichaelBelousov added the performance potential performance increase label Nov 2, 2023
@MichaelBelousov
Copy link
Contributor Author

the federation-guid-optimization branch also adds a new source changes cache which is unbounded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance potential performance increase
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants