Re: [PATCH] x11 terminal respects the requested aspect ratio

[Dima K. <gn...@di...>]

> On Wed, 3 Oct 2012 10:21:19 -0700
> Ethan A Merritt <sf...@us...> wrote:
>
> On Wednesday, October 03, 2012 01:39:08 am Dima Kogan wrote:
> > > On Sat, 29 Sep 2012 17:07:51 -0700
> > > Ethan Merritt <merritt@u.washington.edu> wrote:
> > >
> > > > 6. Removing duplicate messages (such as duplicate consecutive V commands) sounds
> > > >    great. We should do it
> > >   OK. Low priority because it's relatively rare for normal plots.
> > 
> > I did a first pass at this. Patch attached. Good news is that as expected, the
> > traffic drops dramatically. Inboard timing drops from about 0.9s to about 0.45s.
> > The outboard, however, drops from 0.85s to 0.01s! The reason the inboard didn't
> > drop as much is that it still has to parse the original huge data file. I have
> > some lingering concerns about the patch I'm attaching. 
> 
> > I use ftell() to check to see that the V commands are indeed consecutive.
> > This might have a non-negligible cost, so I'd check before committing this.
> 
> That's clever, but I agree that there is potential cost.
> Other terminal drivers do the same job without resorting to ftell().
> The trick is that any command that potentially affects the current
> active position must either update or invalidate the inboard copy
> of x_last and y_last.   For example,  term->put_text() would set
>   x_last = y_last = INVALID;           /* #define INVALID -1 */
> before leaving.
> 
> The down side is that unlike your ftell() version this approach requires
> finding all the places that might affect current position.  I've attached a 
> first-pass patch that catches most of them, but I probably missed some.
> Other terminal drivers can serve as a model.
> 
>  
> > At this point, my test case is clearly broken since we've been able to optimize
> > away all its complexity.
> 
> Right.  I modified your original data generation script to produce longer
> vectors and some gaps, so the the plot would contain move commands as well
> as vector commands.  This gives a more realistic mix of commands:
> 
> perl -e 'for(0..2000000) \
>  { print "$_ " . sin($_/1000) . "\n"; print "\n" if $_ % 100 == 0; }' \
>  > breaks.ascii
> 
> With this test data the reduction in size from removing redundant commands
> is less than 10%.  I tested using the "uniq" command rather than patching 
> the driver source code.  10% max didn't seem very significant to me,
> which was why I said it was low priority.
> 
> > Is the same optimization valid for P commands? 
> 
> I don't think so.  But it does apply to M commands.
> 
> > I'm thinking of just generating a bunch of discrete points, and sending 
> > them over as P commands. That sounds good, right?
> 
> I don't think that the active position after drawing a point symbol is 
> guaranteed to be at the center of the point.  So in the sequence
>   Move(x,y);  Point(x,y);  Move(x,y); Vector(x1,y1);
> the second Move is not redundant.
> 
> Of course, we could change the code so that Point(x,y) it _is_ guaranteed
> to leave the active position at (x,y).  


OK. I revisited this (duplication suppression). Patch attached. I believe this
optimization is equally applicable to P, M, and V. Note that this is all purely
inboard, so there's no active position at all; that's an outboard concept. So
for instance, a duplicated P command would normally draw the same point glyph
multiple times in the same exact position, thus removing the duplication doesn't
change the output. Tell me if I'm misunderstanding.

I took your suggestion to keep track of pipe accesses myself, instead of using
ftell. I'm troubled by the manual maintenance this requires, but the performance
difference looks significant.

The data file you generated in your post had fewer duplicates because it was a
much higher frequency sinusoid, so it was aliasing heavily. I took measurements
from a highly redundant sinusoid and a highly aliased one:


No suppression at all:

|      | /1e6 (1% uniq) | /500 dataset (90% uniq) |
|------+----------------+-------------------------|
| user |           0.91 |                    0.92 |
| sys  |           0.20 |                    0.21 |


Suppression with attached patch:

|      | /1e6 (1% uniq) | /500 dataset (90% uniq) |
|------+----------------+-------------------------|
| user |           0.40 |                    0.89 |
| sys  |           0.14 |                    0.19 |


Suppression with ftell:

|      | /1e6 (1% uniq) | /500 dataset (90% uniq) |
|------+----------------+-------------------------|
| user |           0.46 |                    1.75 |
| sys  |           0.21 |                     4.3 |



Conclusions:

1. ftell() is way too slow
2. the code with the attached patch is significantly faster than before in the
   best case, and about the same in the worst case

Next, I'm going to look into binary communication again.

dima