> > 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 think you are not unstanding what I was trying to say. The issue is not whether
> repeated P commands are redundant, the question is whether or not a M command is
> needed after the P. Scenario: one might think that a series of points connected
> by lines could be drawn as
> P(x1,y1) V(x2,y2) P(x2,y2) V(x3,y3) ...
> But that doesn't work because P(x1,y1) doesn't leave the current position
> at (x1,y1). So instead one needs to do
> P(x1,y1) M(x1,y1) V(x2,y2) P(x2,y2) M(x2,y2) V(x3,y3) ...
> My point is that all the M commands may seem redundant but they are not.
> [NB: This is not the ordering produced by "with linespoints"]
>
> > 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
>
> I've been too busy to have a serious look at your non-redundancy patch,
> but at first glance it looks more complicated than necessary.
> Do you really need to set X11_IPC_LASTDATARUN_NONE for every single
> command that doesn't change the current position?
> Other terminal drivers manage just fine without this.
> If the small set of commands that _do_ change the position (M, V, P, T, ??)
> track the current position then no one else needs to care.
You're right, I wasn't fully understanding what you meant. The optimization
implemented in the patch I attached is a bit simpler than what you're
describing. It only looks for identical, consecutive M, P or V commands, and
suppresses any found duplicates. If ANY command is seen between successive P
commands, say, nothing is suppressed. So for instance,
P 1 2 3
M 3 4 5
P 1 2 3
would NOT suppress anything because there's an M between the two Ps. To make
this optimization I need to remember what the last command was, and its
arguments. The enum is the command type.
To be clear, in the patch I attached, only duplicate commands are removed, thus
the M is always preserved after the P in your example. The optimization you're
describing is more powerful. Do you think there will be a noticeable difference
between the two approaches with real-world data?
The main thing I don't like about the patch is all the X11_IPC_LASTDATARUN_NONE
resets, but I can't think of a better way to eliminate these without resorting
to ftell() or redefining printf(). It's likely that I've gone way overboard on
the resets, and you can get away with them in just the few places you mention
(M, V, P, T, ...).
Let me know what you think. For the record, I did find a way to handle this
without requiring the manual X11_IPC_LASTDATARUN_NONE reset or suffering the
performance hits of doing an ftell. One can redefine the functions that interact
with the pipe:
static int X11_ipc_counter;
static int _fprintf_return, _fputs_return, _fputc_return;
#define fprintf(stream, format, ...) \
( _fprintf_return = fprintf(stream, format, ## __VA_ARGS__ ), \
(( stream == X11_ipc ) && X11_ipc_counter++), \
_fprintf_return )
#define fputs(s, stream) \
( _fputs_return = fputs(s, stream), \
(( stream == X11_ipc ) && X11_ipc_counter++), \
_fputs_return )
#define fputc(c, stream) \
( _fputc_return = fputc(c, stream), \
(( stream == X11_ipc ) && X11_ipc_counter++), \
_fputc_return )
This is sorta weird, but serves to automatically keep track of the pipe
position. The extra code then is localized to the functions that need the
duplicate suppression (X11_vector, X11_point, etc). I.e. no resets are required
anywhere, like with ftell(), and the performance is as good as with the attached
patch. The downsides are that it
1. uses a C99-only construct (__VA_ARGS__ in the preprocessor)
2. uses a gcc-ism: , ## __VA_ARGS__
3. adds an unexpected surprise to anybody looking at the code, since a
plain-looking fprintf() now does more than an uninitiated reader would
expect.
Maybe these are acceptable, or somebody can see a way to work around these
downsides.
dima
|