[haiku-development] Re: My SetViewUI ... patch
- From: looncraz <looncraz@xxxxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Sun, 4 Oct 2015 20:12:15 -0500
On 10/4/2015 15:13, Axel Dörfler wrote:
There is like a gazillion of coding style violations, but other than
that, I'm not too fond of your delayed message implementation.
I tried to follow the styling I found in every file I worked on. You
will need to be more specific for me to address your concerns, but
unless I were to run through every file on the system and do code
styling cleanup, there's not much I can do about the fact that the
existing codebase does not consistently follow the guidelines.
I'm not too sure what you are not fond about, though, would be helpful
if you could be more detailed in that regard. It works well, and I
think it's quite clean. I don't like the DelayedMessageThread
implementation, though, especially since that was originally just a
place-holder, so I just spit it out in a few minutes. I will not be
leaving that alone, that's for sure, and I can understand you wanting to
have me do that before committing a patch.
As far as DelayedMessage itself, it follows the methods used for
LinkSender very closely and is lightweight. A future revision would be
to improve timing by sending the intended time of delivery from the
client rather than the intended delay, that should shave some time off.
I'm not out of ideas, yet ;-)
My goal for now is to get a wider examination and feedback on what has
been done. I noticed an interesting issue with CharacterMap, for
example, so I'll be adding a patch for that as well. And I don't know
how many third party apps may present interesting alternative uses of
native faculties that I did not handle properly.
In addition, I am working to remove the DelayedMessageThread, but I
would like to find a way that doesn't resort to sending extra
messages through the MessageLooper thread (this resulted in stuck
messages and messages being delivered amazingly late).
That sounds more like an error, not like an inherent problem.
Sadly, it seems to just be a result of needing to wakeup the thread
(sending a message to the port) in order to change the timeout when the
next delayed message needs to be fired before and there being a queue of
messages. With the messages getting added to the end of the queue, it
would sometimes take quite a while before it was serviced and one window
would fire its message a frame or two after the others. The end result
was the one window updating noticeably before or after others, and
different values being set until all the messages were flushed. The
timing naturally became staggered, seemingly dependent upon the drawing
complexity of the windows... and the whole idea is to decouple the
system from the application's computational costs - and vice versa.
To prevent that, I had to send an extra message for nearly every
DelayedMessage created, rather than just the ones that needed to fire
before the next scheduled message, which increases server-side CPU usage
(client side message load stayed the same). I also don't like putting
more messages in the looper's queue, either. It is quite possibly I am
missing something in the current implementation, I'll be sure to examine
it more tomorrow (I had a few ideas come to mind earlier today, but no
time to examine it).
http://files.looncraz.net/SetViewUI.patch
http://files.looncraz.net/SetViewUI-patchset.zip
A zip file isn't really that helpful -- why not have the patchset as a
single file? In any case, it shouldn't end up as one big commit in our
codebase.
Okay, I misunderstood what you wanted before, here ya go:
http://files.looncraz.net/SetViewUI-patchset.patch
--The loon
Other related posts:
