two patches

[Tomaz Canabrava]

On Fri, Dec 12, 2014 at 5:38 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
>
> On Fri, Dec 12, 2014 at 05:03:54PM -0200, Tomaz Canabrava wrote:
> > > So I'm puzzled how often we are adding bookmark events and how that
> > > affects performance... or is this really just a first of a series and
> the
> > > others are more performance relevant?
> >
> > First of a series.
>
> OK
>
> > > Dear C expert... I am curious. Why are you doing this with a struct
> event
> > > **
> > > instead of just a struct event *... you need the ** if you want to be
> able
> > > to modify the list in an elegant way, but all you do here is walk the
> > > list, so this could drop one level of indirection... or am I missing
> > > something?
> >
> > Copied from the code just above that one.
>
> But that code modified the list, right?
>
> > > And why add the curly braces, anyway?
> >
> > I was debugging and forgot to remove them.
>
> Tsktsktsk. So you added something because you forgot to remove it and
> introduced whitespace damage that way...
>
> > > This could do something really bad if for some reason
> > > internalEvent->time.seconds is negative, right? then entry[0].sec
> (which
> > > should always be 0) could already be bigger and we access element
> entry[-1]
> >
> > Is that possible?
>
> Off the top of my head I'd say "unlikely", but I worry about dives
> imported from other sources or something else that could break this
> assumption. So I'd rather have you test for i > 0 before you access
> entry[i - 1]
>
> > > So this is pretty invasive and the patch has a few issues.
> > >
> > > Why should this go in before 4.3? Is there a specific bug this
> addresses?
> > >
> >
> > I was trying to find out why when moving a node for a few seconds while
> > adding a new dive used 100% of the CPU in release mode. turned out it was
> > because of too many replots that used the *very* expensive calls to
> > PainterPath.
> > while I was trying to fix that I got carried away.
>
> I hear that happens. So does this mean "I'll rework this for until after
> 4.3" or "I'll rework this to address the issues mentioned and then submit
> the series that fixes the bigger problem later today"?
>

After 4.3 I'll touch this again, you can forget this one.
:)


>
> > I think I'll just
> > generate the pixmap of the Glyphs and reuse them in the future for the
> > PainterPath issue.
>
> Or that and we forget the patch? That seems wrong because the idea in the
> patch seems correct.
>
> /D
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20141212/aaab4791/attachment.html>