Skip to content

Improve performance of query in KisTagResourceModel

The commits in this MR are authored by Grum because they did most of the work. I only made it actually working within Krita. I also changed from max(resourceId) to min(resourceId) when choosing which row to return. I think it makes more sense, considering that the oldest version (when we would mark resources inactive) would use the oldest resources instead of the newer ones.

Test Plan

  1. Prepare a new branch with just the second commit with indeces.
  2. There are four cases: (1) before MR, (2) with new sql query but no new indeces, (4) with indeces but the old sql query, (3) with new sql query and with indeces.
  3. Import Rakurri bundle: https://rakurri.gumroad.com/l/rakurribrushes2?recommended_by=library
  4. Add this patch: grum_performance_improvements_time_counting_complete.diff - it's just a simple timer
  5. For every case:
    • make sure there is no old sql database (important! so every test case has the same chance)
    • go to Settings -> Resource Manager, set the Rakurri bundle as a storage
    • select all resources
    • using the [+] button in the tag widget on the right side, add a tag called "rakurri1"
    • unselect resources, select all of them again, add a tag called "rakurri2" etc. until you have four tags
    • go to console and write down how much time every adding a tag took
    • remove the database

For me, the results were as follows:

  • (1) no new indeces, old sql: 1x as fast, 1.0 of time (baseline)
  • (2) no new indeces, new sql: 2.755x faster than baseline, took only 0.364 of time
  • (3) new indeces, old sql: 1.123x faster than baseline, took 0.893 of time
  • (4) new indeces, new sql: 3.469x faster than baseline, took 0.294 of time (1.26x faster than (2), and 0.807 of time that (2) took)
Query No new indeces New indeces
Old query t 0.893*t (1.123x faster)
New query a = 0.364*t (2.755x faster) 0.294*t (3.469x faster), and 0.807*a (1.26x faster than without indeces)

I made four tags for every test case, and I tested all cases twice with roughly the same conditions on my laptop. Example (fastest) values for (1): 25.721, 60.902, 90.326, 123.611; example (slowest) values for (4): 9.264, 20.753, 23.604, 32.668. My values in the table are made this way: mean(mean(test3./test1)) and mean(mean(test1./test3)) (columns = times for consequtive tags in one test case, rows = repeated test cases).

Note that every time you add a new tag, the time increases a lot. That's because the database gets bigger and bigger. From the values I got (which are consistent with common sense), the bigger the database is, the bigger the advantages of using the faster versions (both new indeces and new query) are.

Indeces only improve the performance a little bit but they are safe to use. The new sql query presents risks (could be incorrect, potentially) but improves the performance a lot.

Grum results were as follows:

Query No new indeces New indeces
Old query 4879ms 3706ms
New query 1481ms 485ms
Returned rows 2645 2645

Questions

  1. Is the sql query really correct and whatnot? I checked the performance thoroughly (because I noticed something weird at some point, fortunately I was incorrect and it is indeed faster ;) ).
  2. Should the commit with indeces change the version of the database? I think yes, because it won't add the indeces otherwise.

NOTE

Further performance improvements will be done (for example, I am working on making tagResource() take multiple resource ids at once).

Rough values from my testing

All in octave form. I believe they are in seconds (yes, it takes so much time, that's why improvements are important there).

test1 = [27.067, 61.884, 92.730, 125.112; 25.721, 60.902, 90.326, 123.611]
test2 = [9.577, 22.319, 31.167, 42.939; 9.915, 24.030, 31.808, 47.805]
test3 = [24.065, 54.508, 79.775, 104.733; 24.987, 57.130, 81.244, 106.822]
test4 = [9.264, 20.753, 23.604, 32.668; 9.092, 17.515, 23.460, 31.431]

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.
Edited by Agata Cacko

Merge request reports