[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-kimageshop
Subject: Re: My meeting notes for 2023-03-20
From: Dmitry Kazakov <dimula73 () gmail ! com>
Date: 2023-03-22 8:29:48
Message-ID: CAEkBSfU9s+iWRpDqsxBGP_HFE=fpHS2Jha+XXKD1CpXa-uptnQ () mail ! gmail ! com
[Download RAW message or body]
On Wed, Mar 22, 2023 at 8:12 AM Sharaf Zaman <shzam@sdf.org> wrote:
>
> So, we just create a map to mark broken preset as "broken" and somehow
> keep them
> in sync with the database (I'll have to investigate how)?
>
Yes, the original implementation did it in somewhat this way, but there was
an issue that this information is not available on resource loading (other
resources could be unavailable). So Halla refactored that into calculation
on the fly.
Ideally, you need to add one more stage to resource initialization. That
stage would happen **after** all the resources are loaded, then you could
update this flag. And you should somehow rerun this phase every time and
resource is added/removed.
> PS: I'm not very well-versed with the dependent-resource system (probably
> evident from the fact that I removed code in delegate rendering :p).
>
> Dmitry Kazakov <dimula73@gmail.com> writes:
>
> > Hi, Sharaf!
> >
> > I have just checked the code in VTune. It seems like the line you
> mentioned takes about 10% time while scrolling. Not that it is too much, but
> > still comparable with 15% spent on the thumbnail scaling :)
> >
> > I guess we have two solutions here:
> >
> > 1) Just refactor the dependent-resource fetching/marking and don't do
> the checks in the delegate. That would remove the whole metadata
> > fetching line from the delegate.
> > 2) Make sure that KoResource::metadata() is always synchronized with the
> database. I guess it should be somewhat synchronized, though I'm
> > not very sure :)
> >
> > On Mon, Mar 20, 2023 at 1:08 PM Dmitry Kazakov <dimula73@gmail.com>
> wrote:
> >
> > HI, Sharaf!
> >
> > Do you mean that you see this metadata line in the profiler when
> rendering/scrolling the preset chooser?
> >
> > I can profile this testcase under VTune and share results if needed.
> >
> > On Mon, Mar 20, 2023 at 12:43 PM Sharaf Zaman <shzam@sdf.org> wrote:
> >
> > Hello!
> >
> > I hope everybody had a good weekend and has been having a good Monday :)
> >
> > To give an update on what I did last week: Firstly I merged most of my
> standing
> > merge requests, I merged my MR to improve rendering of Brush Presets,
> although I
> > had to make some modifications because they were breaking broken
> presets. So, it
> > is next in line for some optimizations.
> >
> > I merged my MR for easier switching of resource locations and I fixed a
> bug with
> > cursor icon being stuck as a "hand" (bug 456183).
> >
> > And finally I looked into caching prepared SQL statements that we use.
> I first
> > asked about it on phabricator (<
> https://phabricator.kde.org/T15245#288770>) and
> > was told nobody has looked into it before. I had noticed that
> QSqlQuery::prepare
> > being shown high up in the profiler, so I thought it might be a good
> idea to
> > optimize this away, but before I do a big search and replace, I want to
> make
> > sure with something more localized to see if it actually can improve
> performance
> > in Krita or not.
> >
> > Kind Regards,
> > Sharaf Zaman
> > <https://www.sh-zam.com>
> >
> > –
> > Dmitry Kazakov
>
--
Dmitry Kazakov
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" \
class="gmail_attr">On Wed, Mar 22, 2023 at 8:12 AM Sharaf Zaman <<a \
href="mailto:shzam@sdf.org">shzam@sdf.org</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><br> So, we just create a map to mark broken preset as \
"broken" and somehow keep them<br> in sync with the database (I'll have to investigate \
how)?<br></blockquote><div><br></div><div>Yes, the original implementation did it in somewhat \
this way, but there was an issue that this information is not available on resource loading \
(other resources could be unavailable). So Halla refactored that into calculation on the fly. \
</div><div><br></div><div>Ideally, you need to add one more stage to resource initialization. \
That stage would happen **after** all the resources are loaded, then you could update this \
flag. And you should somehow rerun this phase every time and resource is \
added/removed.</div><div><br></div><div> <br></div><div><br></div><div><br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><br>
PS: I'm not very well-versed with the dependent-resource system (probably<br>
evident from the fact that I removed code in delegate rendering :p).<br>
<br>
Dmitry Kazakov <<a href="mailto:dimula73@gmail.com" \
target="_blank">dimula73@gmail.com</a>> writes:<br> <br>
> Hi, Sharaf!<br>
><br>
> I have just checked the code in VTune. It seems like the line you mentioned takes about \
10% time while scrolling. Not that it is too much, but<br> > still comparable with 15% spent \
on the thumbnail scaling :)<br> ><br>
> I guess we have two solutions here:<br>
><br>
> 1) Just refactor the dependent-resource fetching/marking and don't do the checks in the \
delegate. That would remove the whole metadata<br> > fetching line from the delegate.<br>
> 2) Make sure that KoResource::metadata() is always synchronized with the database. I guess \
it should be somewhat synchronized, though I'm<br> > not very sure :)<br>
><br>
> On Mon, Mar 20, 2023 at 1:08 PM Dmitry Kazakov <<a href="mailto:dimula73@gmail.com" \
target="_blank">dimula73@gmail.com</a>> wrote:<br> ><br>
> HI, Sharaf!<br>
><br>
> Do you mean that you see this metadata line in the profiler when rendering/scrolling the \
preset chooser?<br> ><br>
> I can profile this testcase under VTune and share results if needed.<br>
><br>
> On Mon, Mar 20, 2023 at 12:43 PM Sharaf Zaman <<a href="mailto:shzam@sdf.org" \
target="_blank">shzam@sdf.org</a>> wrote:<br> ><br>
> Hello!<br>
><br>
> I hope everybody had a good weekend and has been having a good Monday :)<br>
><br>
> To give an update on what I did last week: Firstly I merged most of my standing<br>
> merge requests, I merged my MR to improve rendering of Brush Presets, although I<br>
> had to make some modifications because they were breaking broken presets. So, it<br>
> is next in line for some optimizations.<br>
><br>
> I merged my MR for easier switching of resource locations and I fixed a bug with<br>
> cursor icon being stuck as a "hand" (bug 456183).<br>
><br>
> And finally I looked into caching prepared SQL statements that we use. I first<br>
> asked about it on phabricator (<<a href="https://phabricator.kde.org/T15245#288770" \
rel="noreferrer" target="_blank">https://phabricator.kde.org/T15245#288770</a>>) and<br> \
> was told nobody has looked into it before. I had noticed that QSqlQuery::prepare<br> \
> being shown high up in the profiler, so I thought it might be a good idea to<br> > \
optimize this away, but before I do a big search and replace, I want to make<br> > sure \
with something more localized to see if it actually can improve performance<br> > in Krita \
or not.<br> ><br>
> Kind Regards,<br>
> Sharaf Zaman<br>
> <<a href="https://www.sh-zam.com" rel="noreferrer" \
target="_blank">https://www.sh-zam.com</a>><br> ><br>
> –<br>
> Dmitry Kazakov<br>
</blockquote></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- \
</span><br><div dir="ltr" class="gmail_signature">Dmitry Kazakov</div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic