Several new (or old) feature-branches proposed for upstream merge

[Carl Worth]

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>