Several new (or old) feature-branches proposed for upstream merge
Carl Worth
cworth at cworth.org
Sat Mar 23 02:41:17 PDT 2013
Hi José (and apitrace users),
I've just pushed a new "cworth" branch to the apitrace repository which
is an octopus merge of 6 branches that I am proposing for merging
upstream.
Some of these branches have been seen before (some of which have been
modified or updated with new commits), and some are new. All have been
rebased onto the most recent master branch and tested against the
apitrace-tests repository.
Here's a run-down of the changes in the branches. I think everything
here should be entirely uncontroversial, but for one commit which I'll
call out in more detail below:
remote-target
This is a feature to allow qapitrace to perform its replays on a
remote target via ssh. This commit has been updated according to
feedback from José (to avoid X11 forwarding over ssh so that
the remote X server is tested). The patch has also been tested
by the original user who requested the change and the
documentation has been updated significantly in response to that
testing.
This feature still has some (documented) limitations, such as
the trace file needing to be visible on the remote
system. José's previous suggestion to stream the trace over
stdin is still a viable future improvement. But the CLI will not
need to change for this, and the patch is useful already for the
user originally requesting this feature, so I believe it's ready
to be pushed as-is.
trim-more-no-side-effects
This is a bug-fix series in response to a user who reported that
calls such as glGetInteger* were not being trimmed by:
apitrace trim --auto --trim-spec=no-side-effects
In fact, the list of calls flagged as NO_SIDE_EFFECTS was
dramatically smaller than it should have been, and this series
expands the list significantly, (and to match the state of calls
with siedeefects=False in specs/glapi.py file).
There are also three related bug fixes in this series.
cworth-bug-fixes
This series collects two miscellaneous bug fixes.
One is a bug where passing both --calls and --frames options to
trim would cause the trim code to bailout as soon as one list
was exhausted without continuing to process the other list. I've
added a new test case for this bug in the cworth-bug-fixes
branch pushed to apitrace-tests.
The second bug is a missing glFlush in glretrace so that a user
can use "apitrace replay --wait" to see the results of a trace
even if the user happened to trim away the final glFlush or
glxSwapBuffers call from the original trace.
trim-optimization
This series includes a really important optimization for
trimming which takes it from painfully-slow-as-to-be-unusable to
tolerably-slow on many workloads. I thought I had proposed the
first commit in this series earlier, but I don't recall getting
any feedback on it.
The first commit in this series, (identical to what was submitted
previously), improves trim performance by changing internal
lists to use a much faster data structure (a small skip-list
implementation) instead of std::set<unsigned> which was used
before and was dominating the profiles.
The following three commits in the series are new as of this
submission. They change the code that accepts a call-list on the
"apitrace trim" command line to also use the skip-list (at least
when there is no step or frequency modifier). This resolves a
"TODO" comment in the code regarding switching to a binary tree
to speed up lookups.
For a few manually-entered ranges on the command-line, the
performance of these lookups is not at all significant. But I
ran into heinous performance problems when using a (very long)
call-list generated by "trim --print-callset" into a file and
then "trim --call=@<filename>". This series totally resolves the
performance bugs I hit.
replay-loop
This is a simple feature for "apitrace replay" which adds a
--loop option. This will have the visual appearance of --wait,
(in that the last frame remains visible), but the implementation
is totally different. What it does is continually loop over the
calls of the last frame, replaying them forever.
This is in response to a user who wanted exactly this behavior,
(so that while replaying the frame over and over, the user could
use other tools to investigate what is happening).
The replay does not currently do anything clever, such as
ensuring that the state is correctly restored to what it was
when originally at the beginning of the frame. Instead, it
naively replays the calls of the frame. For the few traces I've
thrown at this, (where, for example, I'm looping over one frame
in the middle of a game where things are already loaded), this
works just fine.
Obviously, we could improve this feature to be more robust in
the future. But I think it's already useful in many case.
trim-auto-by-default
This is the only branch that I think might be controversial.
The first patch in the series adds the --no-deps, --no-prune,
and --exact options as I had originally provided when I first
wrote the trim code. These should be non-controversial. They are
well-defined, and allow for scripts to specify precisely the
behavior they want, (even if, for now, passing these options
doesn't change anything from the default).
The second patch effectively switches the default behavior of
"apitrace trim" from --exact to --auto. My rationale for this is
that only --auto can make sense as a default user interface.
Imagine a new user wanting to trim a trace down to a single
frame. A quick look at "apitrace trim --help" might lead to the
following reasonable command line:
apitrace trim --frames=206
But with the default behavior of --exact, this command will
result in a non-working trace, (and this without even giving the
user any sort of warning that the trim won't be doing what's
expected).
I actually had a user encounter precisely this problem, and then
after encountering the broken resulting trace, the user tried to
debug with:
apitrace trim --trim-spec=drawing --frames=206
But, of course, with --exact still effectively in place, this
command was behaving exactly the same and resulting in the same
broken trace.
This is particularly frustrating to me since
--trim-spec=drawing, (with --auto), is actually very robust. I
have not yet found a trace where it does not work correctly. (If
anyone does have one, pleas share it with me!)
The --exact option does have a use, but it's not in a simple
case like I'm describing here. It's only useful if the user has
somehow arranged for all dependencies to be taken care of, (for
example through a previous call to "trim --print-callset" or
something), or if debugging the trim itself. For these cases
where the user must know more about what's going on, it's
reasonable to expect the user to know to pass --exact as well.
I can think of only one case where a naive user might actually
want something like the --exact behavior by default and that's
in a case like:
apitrace trim --frames=0-206
In this case, (trimming only the end off of the trace), the user
does know that all dependencies will be automatically taken care
of. And it would be faster (and safer) to not do any dependency
analysis. I'd like to treat this as an optimization to
--auto. Even with a --auto default, we could easily detect this
scenario and disable dependency checking. I haven't written that
optimization yet, but I plan to, (and I wouldn't object if you
felt that that optimization needs to be in place before merging
this series).
I also pushed a trim-auto-by-default branch to the
apitrace-tests repository for the one test case that needed a
minor change to track this change in apitrace.
Anyway, that's quite a bit of text for these several changes. I pushed
them each to a separate feature branch so that you can pick and
choose. Obviously I'd be happy to discuss any of the above in
sub-threads as you see fit.
-Carl
PS. This --auto change is in a similar position that the replay
vs. retrace change was in earlier. Namely, I find myself arguing for an
upstream change to revert code to the way I wrote it in the first place,
(but the code was changed between when I wrote it and when it was merged
upstream).
José, I'd like to ask for a process change going forward. If you want to
change something in code I've written, will you please discuss it with
me on the mailing list before changing it and merging? This can save us
both a lot of work, (often I'll be happy to make changes myself if you
ask for them, so you won't have to---and through discussion in advance
we can avoid churn from changing code back after the fact).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20130323/8ebd99ac/attachment.pgp>
More information about the apitrace
mailing list