Hi guys, Although now I'm just an old (ancient?) lurker, and user of Rosegarden (less often than I would like), I really do appreciate your efforts.
Wishing you all the very best, Rich Marschall r...@hydrophones.com > Send Rosegarden-devel mailing list submissions to > rosegarden-devel@lists.sourceforge.net > > To subscribe or unsubscribe via the World Wide Web, visit > https://lists.sourceforge.net/lists/listinfo/rosegarden-devel > or, via email, send a message with subject or body 'help' to > rosegarden-devel-requ...@lists.sourceforge.net > > You can reach the person managing the list at > rosegarden-devel-ow...@lists.sourceforge.net > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of Rosegarden-devel digest..." > > > Today's Topics: > > 1. Excessive construction/destruction of ViewElements (mark_at_yahoo) > 2. Re: Excessive construction/destruction of ViewElements (Ted Felix) > 3. Re: Excessive construction/destruction of ViewElements > (mark_at_yahoo) > > > ---------------------------------------------------------------------- > > Message: 1 > Date: Wed, 6 Apr 2022 14:30:43 -0700 > From: mark_at_yahoo <markr...@yahoo.com> > To: rosegarden-devel@lists.sourceforge.net > Subject: [Rosegarden-devel] Excessive construction/destruction of > ViewElements > Message-ID: <448df58c-5a1d-2fc1-a375-23a69b449...@yahoo.com> > Content-Type: text/plain; charset=utf-8; format=flowed > > Note: This post is going to make me even less popular with the > Rosegarden developer community than I already am. > > My topic for today ... > > There are places in the codebase where ViewElements are constructed only > to be destructed shortly thereafter, sometimes immediately. Is this a > problem? Maybe, maybe not. > > My particular interest is the Matrix Editor. I submitted Bug #1626 and > Merge Request #57 because I found that MatrixElements are very expensive > to construct and destruct. Not so much the objects themselves, but > adding and removing their QAbstractGraphicsShapeItems from the > QGraphicsScene. This is both documented and experimentally observable. > > The solution in Merge Request #57 was to not do the adding/removing in > MatrixElement::reconfigure(), which is where the element's > positions/shapes/sizes/etc are set. Instead, two or more > QAbstractGraphicsShapeItems are permanently maintained by the > MatrixElement object, and reconfigured/hidden/shown as necessary. > > I thought that would be sufficient because I believed MatrixElements > were only constructed when the user created a note, and persisted until > they were erased by her/him. So the constructor still creates and adds > the Qt objects, and the destructor removes them. > > In the process of working on Feature Request #504 I discovered there is > far more "thrash" constructing/destructing MatrixElements than I > thought. For example, Matrix Editor tools such as MatrixPainter > construct a MatrixElement, but also a MatrixInsertionCommand. The latter > triggers a ViewSegment::eventAdded() call (I'm skipping several steps > here) which eventually constructs a new MatrixElement identical to the > first, and the original one is then destructed. > > The above could probably be "fixed" (if fixing is necessary), but a > worse situation happens in ViewSegment::findEvent(). That constructs a > polymorphic ViewElement for the sole purpose of using it to find > matching existing ViewElements bound to the same Event, and then > immediately destructs it. (In an excellent example of good coding > practice, the local variable is appropriately named "dummy".) The issue > is that findEvent() is called several times per note during many Matrix > Editor user interactions, and with lots of notes and expensive > constructors/destructors ... well, the problem is obvious. > > Again, it could be argued that none of this matters. Hardware is very > fast these days, users haven't complained about performance, and I only > did when it impacted me working with many thousands of notes at once. So > maybe the solution is to implement a wait cursor that shows up when > needed and move on to more important bugs (IMO having the program hang > for 15-30 seconds or more with no feedback to the user is unacceptable). > > But as I've written before, I'm an optimization freak -- appropriate > optimization, not premature (that's well and correctly known as the root > of all evil) -- and all of the above bothers me. As an immediate remedy > I'm going to add a static CachedFactory (is that a real design pattern?) > to the MatrixElement class which will keep a vector of > QAbstractGraphicsShapeItems (actually two, one of QGraphicsRectItems > for notes and one of QGraphicsPolygonItems for percussion). When the > MatrixElement constructor (actually the reconfigure() method) needs one > it will pop it off and return it, or if the vector is empty do the > expensive construction and scene::addItem(). Likewise the MatrixElement > destructor will return the items, and the factory will call hide() and > pop them back onto the vector for reuse. > > This will solve a lot of the inefficiency, and maybe that's enough. But > I'm also still thinking about ViewSegment::findEvent(). What if Events > had an std::set of ViewElements that referenced them? Each ViewElement > constructor would register itself and be added to the set, and its > destructor would unregister and it would be removed. Then findEvent() > could just return the set's begin() iterator instead doing the current > equal_range() lookup. The important point is not the lookup, but that > the expensive "dummy" ViewElement object wouldn't be needed. > > I get the feeling that in general the RG architecture frowns on keeping > duplicate copies of data, which is a design philosophy I support. > (There's always the possibility that the copies can get out of sync, > with disastrous results.) But I also temper my support with pragmatism. > RG makes very heavy use of STL design patterns (another thing I like). > But it's cases like this where a full understanding of the implications > and trade-offs need to be considered. > > Note that I never planned on doing this kind of deep surgery on the RG > code. I came here to fix one bug and implement one feature, yet in doing > so I keep falling deeper and deeper down the rabbit hole. I've tried to > stay strictly within the Matrix Editor sources (with the exception of > removing the unneeded menus and buttons in other windows that brought up > the Percussion Editor variant of it I deprecated) but the ideas in this > post go way further. Please understand that I haven't gone looking for > (IMO) problems like the above -- they just seem to find me. > > Comments/reactions/criticisms/suggestions welcome. And I obviously > understand that there's a good chance anything I do (including the merge > requests I've already submitted) will end up living forever only in my > own repository fork. I guess that's OK -- I really like Rosegarden, and > if and when I hammer into the shape I need I plan on using it heavily. > My college advisor used to tell me, "The dark side of ability is > choice". If I didn't have the chops (I claim) to make these changes I'd > have to accept the program as-is, or move to another software package. > Probably would have been a lot easier in the long run. > > > > ------------------------------ > > Message: 2 > Date: Wed, 6 Apr 2022 19:09:07 -0400 > From: Ted Felix <t...@tedfelix.com> > To: rosegarden-devel@lists.sourceforge.net > Subject: Re: [Rosegarden-devel] Excessive construction/destruction of > ViewElements > Message-ID: <901e8982-8874-b66f-03b1-d6a41d4ef...@tedfelix.com> > Content-Type: text/plain; charset=UTF-8; format=flowed > > On 4/6/22 5:30 PM, mark_at_yahoo via Rosegarden-devel wrote: >> Note: This post is going to make me even less popular with the >> Rosegarden developer community than I already am. > > No judgment. I'm just glad you are looking at these things and doing > something about it. There are a lot of problems all throughout rg. > It's not surprising that you've found some. > >> But as I've written before, I'm an optimization freak > > It's one of my specialties as well. So I'm really glad you are > looking into all this. > >> Note that I never planned on doing this kind of deep surgery on the RG >> code. I came here to fix one bug and implement one feature, yet in doing >> so I keep falling deeper and deeper down the rabbit hole. > > That's exactly how I ended up here. > >> And I obviously >> understand that there's a good chance anything I do (including the merge >> requests I've already submitted) will end up living forever only in my >> own repository fork. > > Yeah, sorry about that. I will do my best to bring your changes in, > but it is going to take some time. There is a lot going on these days. > > Ted. > > > > ------------------------------ > > Message: 3 > Date: Thu, 7 Apr 2022 01:15:23 -0700 > From: mark_at_yahoo <markr...@yahoo.com> > To: rosegarden-devel@lists.sourceforge.net > Subject: Re: [Rosegarden-devel] Excessive construction/destruction of > ViewElements > Message-ID: <08c5a622-6dbc-6c07-bbe6-413a0c1e0...@yahoo.com> > Content-Type: text/plain; charset=utf-8; format=flowed > > On 4/6/22 4:09 PM, Ted Felix wrote: >> On 4/6/22 5:30 PM, mark_at_yahoo via Rosegarden-devel wrote: >>> But as I've written before, I'm an optimization freak >> >> ? It's one of my specialties as well.? So I'm really glad you are >> looking into all this. > > Thanks for the encouragement, Ted. I took a quick glance at adding a set > of referencing ViewElements to the Event class and while I was afraid > that would require patching in lots of places it might actually be much > simpler. Maybe just in the base ViewElement constructor, but even if not > there's only NotationElement and MatrixElement that derive from it. I'll > look further, and at other/alternate approaches. > > >>> Note that I never planned on doing this kind of deep surgery on the RG >>> code. I came here to fix one bug and implement one feature, yet in >>> doing so I keep falling deeper and deeper down the rabbit hole. >> >> ? That's exactly how I ended up here. > > Now you're scaring me. ;) I've read your bio on the "authors" page. > Coming up on 12 years now, right? So what you're saying is that > Rosegarden is a gothic horror that lures unsuspecting coders into an > infinite purgatory of improving a Frankenstein monster that will never > be truly finished? ;) > > >>> And I obviously understand that there's a good chance anything I do >>> (including the merge requests I've already submitted) will end up >>> living forever only in my own repository fork. >> >> ? Yeah, sorry about that.? I will do my best to bring your changes in, >> but it is going to take some time.? There is a lot going on these days. > > Not a problem. I just wanted to know that my code will eventually be > reviewed for possible inclusion. If it wasn't going to be I could just > go back to simpler hacking on my own. (When they're only going to affect > me personally I'm much more nonchalant about potential bugs.) > > When you do get around to it, lets coordinate and I can try to refactor > the commits into some kind of logical linear progression of merges. I'll > try to write with that in mind, but the one good thing is that I'll be > running the build from the head of my "all changes combined" branch and > hopefully will catch and fix the inevitable bugs there. > > > > > ------------------------------ > > > > ------------------------------ > > Subject: Digest Footer > > _______________________________________________ > Rosegarden-devel mailing list > Rosegarden-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/rosegarden-devel > > > ------------------------------ > > End of Rosegarden-devel Digest, Vol 163, Issue 1 > ************************************************ > _______________________________________________ Rosegarden-devel mailing list Rosegarden-devel@lists.sourceforge.net - use the link below to unsubscribe https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
