[PATCH] RFC: replay: Fix --loop to work when --singlethread is in effect

[José Fonseca]

I agree.

Jose

On Mon, Apr 15, 2013 at 7:16 PM, Peter Lohrmann <peterl at valvesoftware.com>wrote:

> I think the getNextCall(..) option seems like a good approach.
>  - Peter
>
> -----Original Message-----
> From: apitrace-bounces+peterl=valvesoftware.com at lists.freedesktop.org[mailto:
> apitrace-bounces+peterl=valvesoftware.com at lists.freedesktop.org] On
> Behalf Of Carl Worth
> Sent: Monday, April 15, 2013 11:07 AM
> To: apitrace at lists.freedesktop.org
> Cc: Carl Worth
> Subject: [PATCH] RFC: replay: Fix --loop to work when --singlethread is in
> effect
>
> The original implementation of --loop was made only to the code
> implementing multi-threaded replay, (the RelayRunner class and friends), so
> that when --singlethread is in effect, the --loop option has no effect.
>
> This commit fixes the bug by copying the --loop logic to the
> single-threaded case as well.
>
> Thanks to Peter Lohrmann for reporting the bug.
> ---
>
> NOTE: This commit is ugly in that the implementation of --loop is copied
> to two places in the code. I'd really rather eliminate that copying, so
> this is sent out in some respect as a proof-of-concept, (and as a way to
> help Peter get past the current bug).
>
>
> One option to reduce code duplication would be to push the "if
> (singleThread)" condition down into the lower loop. But my first looks at
> doing that held little promise, (there are many places within the "relay
> race" that would have to be touched up with "if (singleThread)").
>
> Another option would be to have some sort of getNextCall function that
> both the single and multi-threaded code could use and then this function
> could hold all of the logic needed for --loop.
>
> I'm glad to do more of the legwork on the code here, but if someone has a
> strong opinion on what direction to go, I'd be glad to hear it.
>
>  retrace/retrace_main.cpp |   36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/retrace/retrace_main.cpp b/retrace/retrace_main.cpp index
> 0c358e1..fd45e78 100644
> --- a/retrace/retrace_main.cpp
> +++ b/retrace/retrace_main.cpp
> @@ -517,10 +517,42 @@ mainLoop() {
>
>      if (singleThread) {
>          trace::Call *call;
> -        while ((call = parser.parse_call())) {
> +
> +        call = parser.parse_call();
> +
> +        /* If the user wants to loop we need to get a bookmark target. We
> +         * usually get this after replaying a call that ends a frame, but
> +         * for a trace that has only one frame we need to get it at the
> +         * beginning. */
> +        if (loopOnFinish) {
> +            parser.getBookmark(lastFrameStart);
> +        }
> +
> +        do {
> +            bool callEndsFrame = false;
> +            static trace::ParseBookmark frameStart;
> +
> +            if (loopOnFinish && call->flags & trace::CALL_FLAG_END_FRAME)
> {
> +                callEndsFrame = true;
> +                parser.getBookmark(frameStart);
> +            }
> +
>              retraceCall(call);
>              delete call;
> -        };
> +            call = parser.parse_call();
> +
> +            /* Restart last frame if looping is requested. */
> +            if (loopOnFinish) {
> +                if (!call) {
> +                    parser.setBookmark(lastFrameStart);
> +                    call = parser.parse_call();
> +                } else if (callEndsFrame) {
> +                    lastFrameStart = frameStart;
> +                }
> +            }
> +
> +        } while (call);
> +
>          flushRendering();
>      } else {
>          RelayRace race;
> --
> 1.7.10.4
>
> _______________________________________________
> apitrace mailing list
> apitrace at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/apitrace
> _______________________________________________
> apitrace mailing list
> apitrace at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/apitrace
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20130423/2ac8e23b/attachment.html>