Skip to content

Comments

Add special optimization of ExternalSourceAspect and remove some unnecessary code#1106

Draft
khanaffan wants to merge 5 commits intomainfrom
affanK/multiaspect-optimization
Draft

Add special optimization of ExternalSourceAspect and remove some unnecessary code#1106
khanaffan wants to merge 5 commits intomainfrom
affanK/multiaspect-optimization

Conversation

@khanaffan
Copy link
Contributor

@khanaffan khanaffan commented May 7, 2025

Closes: https://github.com/iTwin/itwinjs-backlog/issues/1479

Changes

  • BloomFilter is enabled again. (Need Perf testing if it has any negative effect anymore)
  • Disqualifing ECClassId lookup index for ExternalSourceAspect is removed.
  • Disqualify JOIN expression for ExternalSourceAspect.Element.Id is added.

This query optimization involves ExternalSourceAspect, which is typically slow. A previously mentioned method was suggested to accelerate its performance.

Reason:

ExternalSourceAspect.Element.Id is associated with the index (ix_bis_ElementMultiAspect_fk_bis_ElementOwnsMultiAspects_target), which closely matches Element.Id. Consequently, using this index for looping yields performance akin to a SCAN operation. Therefore, the index should be used as a last resort, not as a primary or secondary option.

The patch identifies the search expression 'ExternalSourceAspect.Element.Id' and appends a '+' to invalidate it. Some earlier optimizations related to this have been eliminated.

  • [ X] Need ECPresentation performance testing as prerequisite to merge. @grigasp

@grigasp
Copy link
Member

grigasp commented May 8, 2025

Need ECPresentation performance testing as prerequisite to merge.

I'll try to find some time for this ASAP, but that'll probably only happen next week.

@grigasp
Copy link
Member

grigasp commented May 29, 2025

Not good... Here're the results:

Test case 4.11 5.0-dev.115 5.0-affan 5.0-affan-analyzed
L iModel 1 10.47 11.20 102.91 93.407
L iModel 2 17.74 19.71 189.73 182.022
L iModel 3 4.00 4.23 86.17 83.358
L iModel 4 21.49 22.39 > 10 min. > 10 min.
L iModel 5 39.91 42.27 > 10 min. > 10 min.
L iModel 6 3.14 3.26 4.53 4.464
L iModel 7 1.15 1.35 5.14 4.537
L iModel 8 21.92 23.74 234.33 311.151
M iModel 1 1.02 1.12 1.78 1.403
M iModel 2 2.41 2.57 8.67 7.657
M iModel 3 5.71 23.58 207.52 191.521
M iModel 4 4.02 4.53 96.53 88.335
M iModel 5 1.53 1.92 2.50 2.289
M iModel 6 3.24 3.75 20.27 18.78
M iModel 7 0.33 0.34 0.34 0.326
M iModel 8 0.13 0.14 0.14 0.151
M iModel 9 1.96 1.80 2.08 2.352
M iModel 10 0.22 0.24 0.27 0.256
M iModel 11 0.97 1.01 1.30 1.354
M iModel 12 0.16 0.15 0.18 0.146
M iModel 13 0.37 0.47 1.18 1.167
M iModel 14 0.69 0.65 0.80 0.771
S iModel 1 0.74 0.65 0.66 0.562
S iModel 2 0.27 0.22 0.24 0.212
S iModel 3 0.05 0.05 0.05 0.042
S iModel 4 0.05 0.05 0.05 0.047
S iModel 5 0.79 0.74 0.72 0.678
S iModel 6 0.06 0.05 0.07 0.048
S iModel 7 0.05 0.05 0.05 0.041
XL iModel 1 98.99 93.36 122.13 59.821
XL iModel 2 85.69 88.85 > 10 min. > 10 min.
XL iModel 3 103.23 118.50 > 10 min. > 10 min.
XL iModel 4 34.07 49.47 > 10 min. > 10 min.
XL iModel 5 55.76 79.31 > 10 min. > 10 min.
XL iModel 6 50.10 72.62 > 10 min. > 10 min.
XL iModel 7 3.54 4.10 > 10 min. > 10 min.
L iModel 9 32.99 36.09 > 10 min. > 10 min.
L iModel 10 7.93 8.38 66.88 24.203
M iModel 15 3.06 3.46 56.83 21.265

This is for a test defined here: https://github.com/iTwin/presentation-performance-tests/blob/d7d9e4db41b2e3b3ac92a636fff3ddfd05614676/src/tests/ContentDescriptor.test.mts#L14. The library runs a number of queries for this request, I haven't investigated further which queries exactly got slower.

@grigasp
Copy link
Member

grigasp commented Jun 26, 2025

@khanaffan, based on our internal discussion, I re-ran the test on the test iModels after running ANALYZE on them, using the addon built from this branch. The results are better in a few cases, but for the most part they're still much worse than what we have in 4.11 - see updated table above.

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.

2 participants