On Mon, 2008-03-03 at 12:48 -0800, Gautam Iyer wrote:
> On Mon, Mar 03, 2008 at 04:35:12PM +0100, Brice Figureau wrote:
>
> > > > I feel that this may not be the proper fix. I suppose that the event
> > > > loop should call rxvt_refresh_vtscr_if_needed more often than it does
> > > > right now.
> > > > Adding an Xevent counter shown that with the regular 10ms quick timeout,
> > > > and doing a text selection, it can process more than 50 X mouse events
> > > > before doing any refresh... Breaking the loop for 5 X events in a row,
> > > > also eliminates the issue...
> > >
> > > That sounds good. Just to be sure I got it right, it boils down to the
> > > following, isn't it?
> > > (with 10 events instead of 5, but that was enough according to my
> > > quick test).
> > >
> > > Index: src/command.c
> > > ===================================================================
> > > --- src/command.c (revision 259)
> > > +++ src/command.c (working copy)
> > > @@ -2355,7 +2355,7 @@
> > > if( select_res > 0 )
> > > {
> > > /* Select succeeded. Check if we have new Xevents first. */
> > > - if( selpage == -1 && XPending( r->Xdisplay ) )
> > > + if( selpage == -1 && XPending( r->Xdisplay ) > 10)
> > > continue;
> > >
> > > /* Read whatever input we can from child fd's*/
> >
> > I used an independant counter instead of relying on XPending (I didn't
> > do X11 programming for almost 15 years, forgot about that one :-)).
> >
> > But your code is not equivalent to mine, mine would have been:
> >
> > - if( selpage == -1 && XPending( r->Xdisplay ) )
> > + if( selpage == -1 && XPending( r->Xdisplay ) <= 10)
My own above snippet is plain wrong (it's an infinite loop if there are
no X events...).
> > Otherwise the behavior in front of a large backlog of Xevents is (like
> > mouse selecting a large text is producing): process all the events
> > except the last 10, waiting 10ms foreach (assuming no output from
> > underlying command) without refreshing. Thus you have refresh latency of
> > "size of backlog" x 10ms.
> >
> > What we want (at least me) is to refresh from time to time while still
> > processing events, so we can achieve this by:
> > * reducing the default select(2) timeout (which means process X11
> > events more often)
>
> This won't (or shouldn't) make any difference. We also poll the Xfd, so
> when any X events are generated, select automatically times out.
Oh, ok that's why Frederik patch is also working. So I think Frederik
solution is even better than my original one :-)
> > * forcing a refresh every "n" X11 events.
>
> This should do the trick, without looking at the ugly mouse code :).
>
> > If you choose the default timeout (10ms) along with 5 events, then you
> > have at worst a refresh every 50ms (20 refresh per second).
> >
> > For a smoother experience, I'd suggest reducing select(2) timeout to
> > 5ms with 5 events for a 40 FPS (that what I'm currently using).
>
> Be warned though: We (or at least me) jumped through quite a few hoops
> to ensure that some "busy" tab producing lots of data (e.g. cat
> /dev/random | base64) will not affect other tabs. Further, if the active
> tab is producing copious amounts of data, and the user has Xft + AA +
> pseudo transparency + no DRM under X, and perhaps a really slow
> machine, then we shouldn't gobble 100% of the CPU with screen refreshes.
I just checked and:
- if( selpage == -1 && XPending( r->Xdisplay ) )
+ if( selpage == -1 && XPending( r->Xdisplay ) > 25)
seems to yield the best performance (with the various scroll one-liner,
like "for line in $(seq 200000); do; echo "blah blah blah blah $line";
done"), while still achieving the mouse selection low latency we want.
Granted, it's on my machine with default refreshLimit/skipPages and
without transparency (a 4 year old P4 2GHz, with an Nvidia graphic
card), so YMMV.
> I choose to delay screen refreshes via --refreshLimit and --skipPages
> (replacing the old --jumpScroll behaviour).
>
> But following your suggestion, perhaps the correct thing to do here
> would be to add another "time limit" (either option, or #ifdef in
> feature.h) which decides an interval after which screen refreshes are
> performed, provided it is needed due to *user* interaction (i.e. X
> events). This would involve also checking that the amount of data
> produced by the active tab is smaller than --refreshLimit.
>
> This might get tricky, since requesting screen refreshes (IIRC) resets
> the values of the # chars produced... Thus if we're not careful, a busy
> active tab, combined with lots of mouse movement would cause mrxvt to
> gobble 100% CPU.
What's good with Frederik's code is that is there is no X activity, you
get the current mrxvt behavior. Only if there is activity, then it will
refresh a little bit more often.
I'll run with this hardcoded settings for a while to see if that works
fine, if I get some free time, I'll try to profile the number of screen
refresh we get in some scroll workload combined with mouse actions...
Thanks,
--
Brice Figureau <[EMAIL PROTECTED]>
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Materm-devel mailing list
Materm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/materm-devel
mrxvt home page: http://materm.sourceforge.net
