Performance improvements plans for Resource Rewrite
Open, Needs TriagePublic

Description

Tasks to do

Things to check

  • whether the order is correct
  • check begin... etc. is correct for tagging (preferably in a unit test)
  • check begin... etc. is correct for untagging (preferably in a unit test)
  • check that bug about tagging
tymond created this task.Jan 28 2022, 4:19 PM
tymond updated the task description. (Show Details)
tymond updated the task description. (Show Details)Feb 15 2022, 8:19 PM
sh-zam added a subscriber: sh-zam.Mar 9 2023, 9:57 AM

Hi!

A thing I found out when I was profiling the code for https://invent.kde.org/graphics/krita/-/merge_requests/1731, is the QSqlQuery::prepare being a bottleneck. I looked into a docs a bit and found out that prepared queries are "compiled" by sqlite into whatever its internal bytecode can understand, so this is something which doesn't need to happen over and over again, but something we can cache and only bind values whenever needed (I think Qt's QSqlQuery::exec takes care of resetting the bindings and re-executing the same prepared query).

I'm not sure if this is something which was looked into before, but let me know if you or someone else has any idea about this?

I don't recall QSqlQuery::prepare ever being discussed, so it's likely no one ever noticed. If you have an idea of how to cache it, give it a try?

sure, I'll look into it then :)

If you, say for example, want to insert multiple records in a loop, you would prepare the statement once, then reuse it by binding different values and then executing it in the loop. But I'm guessing the cases that show up in your profiling is more complex than simple loops and would require persisting the QSqlQuery in global scope to fix.

Do you have an idea which queries are the worst offender?

sh-zam added a comment.EditedMar 9 2023, 10:26 AM

IIRC, it was KisResourceCacheDb::metaDataForId where lot of time was spent in prepare().

Do note, this function was called quite frequently, which is probably what drove my attention towards prepare. Even though I refactored out the caller of the function, there could still be some benefits to caching or maybe there won't (since, query could take up some memory as it won't release its resources) :)

The offender in question: https://invent.kde.org/graphics/krita/-/merge_requests/1731/diffs#cee5870c2d096c362ee5fcff64074172b49df69c_94_95

You should call QSqlQuery::finish to release the result cursor and stuff. The prepared statement itself shouldn't take much memory.