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

[GTK] Speedup TreeItem retrieval by removing redundant iterations #882 #904

Merged

Conversation

basilevs
Copy link
Contributor

@basilevs basilevs commented Nov 26, 2023

This is a follow-up to #903
This is a second optimization for #882.

TreeItem and Tree have been checking indexes for bounds during indexed access and traversal (GTK.gtk_tree_model_iter_n_children()) . Such checks are slow as node size computation is an O(N) operation.

As Tree already does iteration of O(N) complexity when retrieving a TreeItem by its index, and such iteration detects out-of-bounds problems, preliminary model bounds check is redundant. Also, bounds check is avoidable during iteration over a node - instead of an index, GTK iterator applies directly.

Performance improvement proved by running org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree.jfaceReveal().

In test jfaceReveal[Shape: STAR, virtual: true]:
10_000 elements: 1_523_338_044ns -> 654_307_337ns (-57%)
100_000 elements: 262_129_718_282ns -> 92_328_764_928ns (-65%)

Raw measurements Timings before the change:
setForeground[Shape: STAR, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 435754849.000000 ns
Time for 100000 elements: 32930055467.000000 ns
Ratio: 7.557014
Degree: 1.878350

jfaceReveal[Shape: STAR, virtual: false]
Execution time should grow as 2.000000 degree polynom. 
Time for 10000 elements: 282842310.000000 ns
Time for 100000 elements: 17676761285.000000 ns
Ratio: 6.249688
Degree: 1.795858

showItem[Shape: STAR, virtual: false]
Execution time should grow as 1.900000 degree polynom. 
Time for 10000 elements: 280539616.000000 ns
Time for 100000 elements: 17282639987.000000 ns
Ratio: 6.160499
Degree: 1.789616

build[Shape: STAR, virtual: false]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 628988573.000000 ns
Time for 100000 elements: 61514983876.000000 ns
Ratio: 9.779984
Degree: 1.990338

secondTraverse[Shape: STAR, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 3803257.000000 ns
Time for 100000 elements: 18826476.000000 ns
Ratio: 0.495009
Degree: 0.694613

traverse[Shape: STAR, virtual: false]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 4462970.000000 ns
Time for 100000 elements: 17298203.000000 ns
Ratio: 0.387594
Degree: 0.588377

getForeground[Shape: STAR, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 5787027.000000 ns
Time for 100000 elements: 42292862.000000 ns
Ratio: 0.730822
Degree: 0.863812

setText[Shape: STAR, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 486632388.000000 ns
Time for 100000 elements: 33522398446.000000 ns
Ratio: 6.888649
Degree: 1.838134

setForeground[Shape: STAR, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 405605653.000000 ns
Time for 100000 elements: 93197010286.000000 ns
Ratio: 22.977246
Degree: 2.361298

jfaceReveal[Shape: STAR, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 1523338044.000000 ns
Time for 100000 elements: 262129718282.000000 ns
Ratio: 17.207587
Degree: 2.235720

showItem[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 4146927.000000 ns
Time for 100000 elements: 49263463.000000 ns
Ratio: 1.187951
Degree: 1.074799

build[Shape: STAR, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 7649109.000000 ns
Time for 100000 elements: 72623828.000000 ns
Ratio: 0.949442
Degree: 0.977468

secondTraverse[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 311968084.000000 ns
Time for 100000 elements: 44995794866.000000 ns
Ratio: 14.423205
Degree: 2.159062

traverse[Shape: STAR, virtual: true]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 3419116562.000000 ns
Time for 100000 elements: 480580461364.000000 ns
Ratio: 14.055691
Degree: 2.147852

getForeground[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 386103619.000000 ns
Time for 100000 elements: 91866083271.000000 ns
Ratio: 23.793116
Degree: 2.376451

setText[Shape: STAR, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 4191921294.000000 ns
Time for 100000 elements: 565903136110.000000 ns
Ratio: 13.499851
Degree: 2.130329

setForeground[Shape: BINARY, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 166237620.000000 ns
Time for 100000 elements: 1670368852.000000 ns
Ratio: 1.004808
Degree: 1.002083

jfaceReveal[Shape: BINARY, virtual: false]
Execution time should grow as 2.000000 degree polynom. 
Time for 10000 elements: 638750.000000 ns
Time for 100000 elements: 660000.000000 ns
Ratio: 0.103327
Degree: 0.014213

showItem[Shape: BINARY, virtual: false]
Execution time should grow as 1.900000 degree polynom. 
Time for 10000 elements: 541208.000000 ns
Time for 100000 elements: 601500.000000 ns
Ratio: 0.111140
Degree: 0.045871

build[Shape: BINARY, virtual: false]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 53576638.000000 ns
Time for 100000 elements: 469564492.000000 ns
Ratio: 0.876435
Degree: 0.942720

secondTraverse[Shape: BINARY, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 6340044.000000 ns
Time for 100000 elements: 72020103.000000 ns
Ratio: 1.135956
Degree: 1.055361

traverse[Shape: BINARY, virtual: false]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 9934545.000000 ns
Time for 100000 elements: 62696808.000000 ns
Ratio: 0.631099
Degree: 0.800097

getForeground[Shape: BINARY, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 8311336.000000 ns
Time for 100000 elements: 117588992.000000 ns
Ratio: 1.414803
Degree: 1.150696

setText[Shape: BINARY, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 182760803.000000 ns
Time for 100000 elements: 1728364148.000000 ns
Ratio: 0.945697
Degree: 0.975752

setForeground[Shape: BINARY, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 29020717.000000 ns
Time for 100000 elements: 217693886.000000 ns
Ratio: 0.750133
Degree: 0.875138

jfaceReveal[Shape: BINARY, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 156459.000000 ns
Time for 100000 elements: 201041.000000 ns
Ratio: 0.128494
Degree: 0.108884

showItem[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 142666.000000 ns
Time for 100000 elements: 761833.000000 ns
Ratio: 0.533998
Degree: 0.727539

build[Shape: BINARY, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 1058834.000000 ns
Time for 100000 elements: 1810917.000000 ns
Ratio: 0.171029
Degree: 0.233071

secondTraverse[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 4419169.000000 ns
Time for 100000 elements: 42824108.000000 ns
Ratio: 0.969053
Degree: 0.986348

traverse[Shape: BINARY, virtual: true]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 76684293.000000 ns
Time for 100000 elements: 710193485.000000 ns
Ratio: 0.926126
Degree: 0.966670

getForeground[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 12724716.000000 ns
Time for 100000 elements: 96851816.000000 ns
Ratio: 0.761131
Degree: 0.881460

setText[Shape: BINARY, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 924185691.000000 ns
Time for 100000 elements: 12315679732.000000 ns
Ratio: 1.332598
Degree: 1.124699

Timings after the change:

setForeground[Shape: STAR, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 413904953.000000 ns
Time for 100000 elements: 32218751959.000000 ns
Ratio: 7.784094
Degree: 1.891208

jfaceReveal[Shape: STAR, virtual: false]
Execution time should grow as 2.000000 degree polynom. 
Time for 10000 elements: 280156938.000000 ns
Time for 100000 elements: 16828164887.000000 ns
Ratio: 6.006692
Degree: 1.778635

showItem[Shape: STAR, virtual: false]
Execution time should grow as 1.900000 degree polynom. 
Time for 10000 elements: 273404653.000000 ns
Time for 100000 elements: 17070857808.000000 ns
Ratio: 6.243807
Degree: 1.795449

build[Shape: STAR, virtual: false]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 592992235.000000 ns
Time for 100000 elements: 59773874902.000000 ns
Ratio: 10.080043
Degree: 2.003462

secondTraverse[Shape: STAR, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 4311882.000000 ns
Time for 100000 elements: 24607299.000000 ns
Ratio: 0.570686
Degree: 0.756397

traverse[Shape: STAR, virtual: false]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 8011140.000000 ns
Time for 100000 elements: 21195506.000000 ns
Ratio: 0.264575
Degree: 0.422549

dispose[Shape: STAR, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 6909724.000000 ns
Time for 100000 elements: 53272391.000000 ns
Ratio: 0.770977
Degree: 0.887041

getForeground[Shape: STAR, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 7120144.000000 ns
Time for 100000 elements: 60535721.000000 ns
Ratio: 0.850204
Degree: 0.929523

setText[Shape: STAR, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 491881041.000000 ns
Time for 100000 elements: 36376546601.000000 ns
Ratio: 7.395395
Degree: 1.868961

setForeground[Shape: STAR, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 215603838.000000 ns
Time for 100000 elements: 52700520158.000000 ns
Ratio: 24.443220
Degree: 2.388158

jfaceReveal[Shape: STAR, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 654307337.000000 ns
Time for 100000 elements: 92328764928.000000 ns
Ratio: 14.110917
Degree: 2.149555

showItem[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 7773241.000000 ns
Time for 100000 elements: 53396056.000000 ns
Ratio: 0.686921
Degree: 0.836907

build[Shape: STAR, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 7015487.000000 ns
Time for 100000 elements: 66910947.000000 ns
Ratio: 0.953761
Degree: 0.979439

secondTraverse[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 2226218.000000 ns
Time for 100000 elements: 22053147.000000 ns
Ratio: 0.990610
Degree: 0.995903

traverse[Shape: STAR, virtual: true]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 2938910067.000000 ns
Time for 100000 elements: 413532179401.000000 ns
Ratio: 14.070937
Degree: 2.148323

dispose[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 6947642.000000 ns
Time for 100000 elements: 67341885.000000 ns
Ratio: 0.969277
Degree: 0.986448

getForeground[Shape: STAR, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 7624534.000000 ns
Time for 100000 elements: 51383072.000000 ns
Ratio: 0.673918
Degree: 0.828607

setText[Shape: STAR, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 2221487313.000000 ns
Time for 100000 elements: 385223363495.000000 ns
Ratio: 17.340786
Degree: 2.239069

setForeground[Shape: BINARY, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 166152959.000000 ns
Time for 100000 elements: 1618188822.000000 ns
Ratio: 0.973915
Degree: 0.988521

jfaceReveal[Shape: BINARY, virtual: false]
Execution time should grow as 2.000000 degree polynom. 
Time for 10000 elements: 414832.000000 ns
Time for 100000 elements: 576248.000000 ns
Ratio: 0.138911
Degree: 0.142737

showItem[Shape: BINARY, virtual: false]
Execution time should grow as 1.900000 degree polynom. 
Time for 10000 elements: 564415.000000 ns
Time for 100000 elements: 612832.000000 ns
Ratio: 0.108578
Degree: 0.035743

build[Shape: BINARY, virtual: false]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 52435649.000000 ns
Time for 100000 elements: 461121747.000000 ns
Ratio: 0.879405
Degree: 0.944189

secondTraverse[Shape: BINARY, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 10646804.000000 ns
Time for 100000 elements: 58443801.000000 ns
Ratio: 0.548933
Degree: 0.739519

traverse[Shape: BINARY, virtual: false]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 7943645.000000 ns
Time for 100000 elements: 71558724.000000 ns
Ratio: 0.900830
Degree: 0.954643

dispose[Shape: BINARY, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 8815393.000000 ns
Time for 100000 elements: 93832624.000000 ns
Ratio: 1.064418
Degree: 1.027112

getForeground[Shape: BINARY, virtual: false]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 7654688.000000 ns
Time for 100000 elements: 92453755.000000 ns
Ratio: 1.207806
Degree: 1.081997

setText[Shape: BINARY, virtual: false]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 175059829.000000 ns
Time for 100000 elements: 1703109327.000000 ns
Ratio: 0.972873
Degree: 0.988056

setForeground[Shape: BINARY, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 25005935.000000 ns
Time for 100000 elements: 200638844.000000 ns
Ratio: 0.802365
Degree: 0.904372

jfaceReveal[Shape: BINARY, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 358749.000000 ns
Time for 100000 elements: 216041.000000 ns
Ratio: 0.060221
Degree: -0.220255

showItem[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 128083.000000 ns
Time for 100000 elements: 229750.000000 ns
Ratio: 0.179376
Degree: 0.253764

build[Shape: BINARY, virtual: true]
Execution time should grow as 1.100000 degree polynom. 
Time for 10000 elements: 1410580.000000 ns
Time for 100000 elements: 1254706.000000 ns
Ratio: 0.088950
Degree: -0.050856

secondTraverse[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 3798574.000000 ns
Time for 100000 elements: 47877891.000000 ns
Ratio: 1.260417
Degree: 1.100514

traverse[Shape: BINARY, virtual: true]
Execution time should grow as 2.100000 degree polynom. 
Time for 10000 elements: 69804134.000000 ns
Time for 100000 elements: 709958515.000000 ns
Ratio: 1.017072
Degree: 1.007352

dispose[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 13976552.000000 ns
Time for 100000 elements: 132821179.000000 ns
Ratio: 0.950314
Degree: 0.977867

getForeground[Shape: BINARY, virtual: true]
Execution time should grow as 1.200000 degree polynom. 
Time for 10000 elements: 10182895.000000 ns
Time for 100000 elements: 107370083.000000 ns
Ratio: 1.054416
Degree: 1.023012

setText[Shape: BINARY, virtual: true]
Execution time should grow as 1.300000 degree polynom. 
Time for 10000 elements: 868622671.000000 ns
Time for 100000 elements: 11341237795.000000 ns
Ratio: 1.305658
Degree: 1.115829

Copy link
Contributor

github-actions bot commented Nov 26, 2023

Test Results

     302 files  +  3       302 suites  +3   18m 20s ⏱️ + 12m 43s
  4 103 tests +  8    4 095 ✔️ +  8    8 💤 ±0  0 ±0 
12 221 runs  +24  12 147 ✔️ +23  74 💤 +1  0 ±0 

Results for commit da365f1. ± Comparison against base commit 28d65a0.

♻️ This comment has been updated with latest results.

@basilevs basilevs marked this pull request as ready for review November 27, 2023 05:34
@laeubi laeubi added the performance Performance issue label Nov 27, 2023
@basilevs basilevs force-pushed the issue_882_removeCountCheck branch 5 times, most recently from 1eed277 to cc810ea Compare November 28, 2023 20:07
@basilevs basilevs changed the title [GTK] Speedup TreeItem retrieval by removing redundant checks #882 [GTK] Speedup TreeItem retrieval by removing redundant iterations #882 Dec 3, 2023
@basilevs basilevs force-pushed the issue_882_removeCountCheck branch from cc810ea to 56369ba Compare December 3, 2023 08:54
@basilevs
Copy link
Contributor Author

basilevs commented Dec 3, 2023

@SyntevoAlex please take a look, if possible.

…ipse-platform#882

TreeItem and Tree have been checking indexes for bounds during indexed
access and traversal. Such checks are slow as node size computation is
an O(N) operation.

As Tree already does iteration when retrieving a TreeItem by its index,
and such iteration detects out-of-bounds problems, preliminary model
bounds check is redundant. Also, bounds check is avoidable during
iteration over a node - instead of an index, GTK iterator applies
directly.

Performance improvement proved by running
org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree.jfaceReveal().

In test jfaceReveal[Shape: STAR, virtual: true]:
10_000 elements: 501_378_662ns -> 218_583_468ns
100_000 elements: 52_781_899_817ns -> 20_872_245_796ns (-60%)
@basilevs basilevs force-pushed the issue_882_removeCountCheck branch from 56369ba to da365f1 Compare December 3, 2023 12:11
@SyntevoAlex
Copy link
Member

I can review it a few days later.
Feel free to find another reviewer if you don't want to wait. But yes, it's not going to be too easy

@basilevs
Copy link
Contributor Author

basilevs commented Dec 3, 2023

Feel free to find another reviewer if you don't want to wait. But yes, it's not going to be too easy

I think you are the only reviewer left, given @akurtakov is reluctant and lack of other recent contributors in Tree.java

Anyway, this is not a critical bug, so please take your time.

@akurtakov
Copy link
Member

FWIW, I am not 100% out but non-UI things have far higher priority for me.

@SyntevoAlex
Copy link
Member

I feel that #930 is significantly more important, so I'll have to handle it first.

@basilevs
Copy link
Contributor Author

@SyntevoAlex are there any news?

@SyntevoAlex
Copy link
Member

Sorry, too many things to do and too little time.
I guess I'll try next week.

@SyntevoAlex
Copy link
Member

This change makes sense to me.
It's somewhat non-trivial, especially around removing SWT.VIRTUAL branch, but it looks correct to me.

@SyntevoAlex SyntevoAlex merged commit 42dd695 into eclipse-platform:master Jan 7, 2024
13 checks passed
@SyntevoAlex
Copy link
Member

In master now, thanks for your contribution!

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

Successfully merging this pull request may close these issues.

4 participants