]> git.cworth.org Git - apitrace/commitdiff
trim: Add a new --trim-spec option for fine-grained trimming
authorCarl Worth <cworth@cworth.org>
Thu, 15 Nov 2012 04:55:31 +0000 (20:55 -0800)
committerJosé Fonseca <jfonseca@vmware.com>
Wed, 6 Feb 2013 15:46:13 +0000 (15:46 +0000)
This has two benefits:

1. In cases where the dependency analysis goes wrong, using the --trim-spec
   option can help identify which portion of the code is misbehaving, (and
   allows the user to still get some trim benefit by avoiding the broken
   code).

2. The implementation of this change breaks the analysis code up into several
   more fine-grained functions, so it should be easier to review and maintain.

cli/cli_trim.cpp
cli/trace_analyzer.cpp
cli/trace_analyzer.hpp

index c5b018856d692c5bf08673bf066d2c7a956c822b..5625a2c4f845ee2e0bccdba1cd24ed42c04b3ddb 100644 (file)
@@ -57,6 +57,7 @@ usage(void)
         "    -a, --auto               Trim automatically to calls specified in --calls/--frames\n"
         "                             Equivalent to both --deps and --prune\n"
         "        --print-callset      Print the final set of calls included in output\n"
+        "        --trim-spec=SPEC     Only performing trimming as described in SPEC\n"
         "        --thread=THREAD_ID   Only retain calls from specified thread\n"
         "    -o, --output=TRACE_FILE  Output trace file\n"
     ;
@@ -89,9 +90,21 @@ help()
         "\n"
         "        --print-callset      Print to stdout the final set of calls included\n"
         "                             in the trim output. This can be useful for\n"
-        "                             tweaking trimmed callset from --auto on the\n"
+        "                             tweaking the trimmed callset from --auto on the\n"
         "                             command-line.\n"
         "                             Use --calls=@FILE to read callset from a file.\n"
+        "        --trim-spec=SPEC     Specifies which classes of calls will be trimmed.\n"
+        "                             This option only has an effect if dependency\n"
+        "                             analysis is enabled. The argument is a comma-\n"
+        "                             separated list of names from the following:\n"
+        "\n"
+       "                               no-side-effects  Calls with no side effects\n"
+       "                               textures         Calls to setup unused textures\n"
+       "                               shaders          Calls to setup unused shaders\n"
+        "                               drawing          Calls that draw\n"
+        "\n"
+        "                             The default trim specification includes all of\n"
+        "                             the above, (as much as possible will be trimmed).\n"
         "\n"
         "        --thread=THREAD_ID   Only retain calls from specified thread\n"
         "\n"
@@ -107,6 +120,7 @@ enum {
     PRUNE_OPT,
     THREAD_OPT,
     PRINT_CALLSET_OPT,
+    TRIM_SPEC_OPT
 };
 
 const static char *
@@ -123,6 +137,7 @@ longOptions[] = {
     {"thread", required_argument, 0, THREAD_OPT},
     {"output", required_argument, 0, 'o'},
     {"print-callset", no_argument, 0, PRINT_CALLSET_OPT},
+    {"trim-spec", required_argument, 0, TRIM_SPEC_OPT},
     {0, 0, 0, 0}
 };
 
@@ -153,6 +168,9 @@ struct trim_options {
 
     /* Print resulting callset */
     int print_callset;
+
+    /* What kind of trimming to perform. */
+    TrimFlags trim_flags;
 };
 
 static int
@@ -160,7 +178,7 @@ trim_trace(const char *filename, struct trim_options *options)
 {
     trace::ParseBookmark beginning;
     trace::Parser p;
-    TraceAnalyzer analyzer;
+    TraceAnalyzer analyzer(options->trim_flags);
     std::set<unsigned> *required;
     unsigned frame;
     int call_range_first, call_range_last;
@@ -288,6 +306,42 @@ trim_trace(const char *filename, struct trim_options *options)
     return 0;
 }
 
+static int
+parse_trim_spec(const char *trim_spec, TrimFlags *flags)
+{
+    std::string spec(trim_spec), word;
+    size_t start = 0, comma = 0;
+    *flags = 0;
+
+    while (start < spec.size()) {
+        comma = spec.find(',', start);
+
+        if (comma == std::string::npos)
+            word = std::string(spec, start);
+        else
+            word = std::string(spec, start, comma - start);
+
+        if (strcmp(word.c_str(), "no-side-effects") == 0)
+            *flags |= TRIM_FLAG_NO_SIDE_EFFECTS;
+        else if (strcmp(word.c_str(), "textures") == 0)
+            *flags |= TRIM_FLAG_TEXTURES;
+        else if (strcmp(word.c_str(), "shaders") == 0)
+            *flags |= TRIM_FLAG_SHADERS;
+        else if (strcmp(word.c_str(), "drawing") == 0)
+            *flags |= TRIM_FLAG_DRAWING;
+        else {
+            return 1;
+        }
+
+        if (comma == std::string::npos)
+            break;
+
+        start = comma + 1;
+    }
+
+    return 0;
+}
+
 static int
 command(int argc, char *argv[])
 {
@@ -300,6 +354,7 @@ command(int argc, char *argv[])
     options.output = "";
     options.thread = -1;
     options.print_callset = 0;
+    options.trim_flags = -1;
 
     int opt;
     while ((opt = getopt_long(argc, argv, shortOptions, longOptions, NULL)) != -1) {
@@ -332,6 +387,13 @@ command(int argc, char *argv[])
         case PRINT_CALLSET_OPT:
             options.print_callset = 1;
             break;
+        case TRIM_SPEC_OPT:
+            if (parse_trim_spec(optarg, &options.trim_flags)) {
+                std::cerr << "error: illegal value for trim-spec: " << optarg << "\n";
+                std::cerr << "See \"apitrace help trim\" for help.\n";
+                return 1;
+            }
+            break;
         default:
             std::cerr << "error: unexpected option `" << opt << "`\n";
             usage();
index 9a4553edc1895418ea9b745ad4a9562a34dd986d..48153f02a50cdd831861dd7c064b0c2899368cd8 100644 (file)
@@ -263,48 +263,27 @@ TraceAnalyzer::stateTrackPostCall(trace::Call *call)
     }
 }
 
-void
-TraceAnalyzer::recordSideEffects(trace::Call *call)
+bool
+TraceAnalyzer::callHasNoSideEffects(trace::Call *call, const char *name)
 {
-
-    const char *name = call->name();
-
-    /* Handle display lists before any other processing. */
-
-    /* FIXME: If we encode the list of commands that are executed
-     * immediately (as opposed to those that are compiled into a
-     * display list) then we could generate a "display-list-X"
-     * resource just as we do for "texture-X" resources and only
-     * emit it in the trace if a glCallList(X) is emitted. For
-     * now, simply punt and include anything within glNewList and
-     * glEndList in the trim output. This guarantees that display
-     * lists will work, but does not trim out unused display
-     * lists. */
-    if (insideNewEndList != 0) {
-        provide("state", call->no);
-
-        /* Also, any texture bound inside a display list is
-         * conservatively considered required. */
-        if (strcmp(name, "glBindTexture") == 0) {
-            GLuint texture = call->arg(1).toUInt();
-
-            linkf("state", "texture-", texture);
-        }
-
-        return;
-    }
-
     /* If call is flagged as no side effects, then we are done here. */
     if (call->flags & trace::CALL_FLAG_NO_SIDE_EFFECTS) {
-        return;
+        return true;
     }
 
     /* Similarly, swap-buffers calls don't have interesting side effects. */
     if (call->flags & trace::CALL_FLAG_SWAP_RENDERTARGET &&
         call->flags & trace::CALL_FLAG_END_FRAME) {
-        return;
+        return true;
     }
 
+    /* Not known as a no-side-effect call. Return false for more analysis. */
+    return false;
+}
+
+bool
+TraceAnalyzer::recordTextureSideEffects(trace::Call *call, const char *name)
+{
     if (strcmp(name, "glGenTextures") == 0) {
         const trace::Array *textures = dynamic_cast<const trace::Array *>(&call->arg(1));
         size_t i;
@@ -316,7 +295,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
                 providef("texture-", texture, call->no);
             }
         }
-        return;
+        return true;
     }
 
     /* FIXME: When we start tracking framebuffer objects as their own
@@ -330,7 +309,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         linkf("render-state", "texture-", texture);
 
-        required.insert(call->no);
+        provide("state", call->no);
     }
 
     if (strcmp(name, "glBindTexture") == 0) {
@@ -365,10 +344,10 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
          * conservative and don't trim. */
         provide("state", call->no);
 
-        return;
+        return true;
     }
 
-    /* FIXME: Need to handle glMultTetImage and friends. */
+    /* FIXME: Need to handle glMultiTexImage and friends. */
     if (STRNCMP_LITERAL(name, "glTexImage") == 0 ||
         STRNCMP_LITERAL(name, "glTexSubImage") == 0 ||
         STRNCMP_LITERAL(name, "glCopyTexImage") == 0 ||
@@ -398,7 +377,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
             }
         }
 
-        return;
+        return true;
     }
 
     if (strcmp(name, "glEnable") == 0) {
@@ -419,7 +398,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
         }
 
         provide("state", call->no);
-        return;
+        return true;
     }
 
     if (strcmp(name, "glDisable") == 0) {
@@ -440,15 +419,22 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
         }
 
         provide("state", call->no);
-        return;
+        return true;
     }
 
+    /* No known texture-related side effects. Return false for more analysis. */
+    return false;
+}
+
+bool
+TraceAnalyzer::recordShaderSideEffects(trace::Call *call, const char *name)
+{
     if (strcmp(name, "glCreateShader") == 0 ||
         strcmp(name, "glCreateShaderObjectARB") == 0) {
 
         GLuint shader = call->ret->toUInt();
         providef("shader-", shader, call->no);
-        return;
+        return true;
     }
 
     if (strcmp(name, "glShaderSource") == 0 ||
@@ -460,7 +446,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         GLuint shader = call->arg(0).toUInt();
         providef("shader-", shader, call->no);
-        return;
+        return true;
     }
 
     if (strcmp(name, "glCreateProgram") == 0 ||
@@ -468,7 +454,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         GLuint program = call->ret->toUInt();
         providef("program-", program, call->no);
-        return;
+        return true;
     }
 
     if (strcmp(name, "glAttachShader") == 0 ||
@@ -486,7 +472,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
         link(ss_program.str(), ss_shader.str());
         provide(ss_program.str(), call->no);
 
-        return;
+        return true;
     }
 
     if (strcmp(name, "glDetachShader") == 0 ||
@@ -503,7 +489,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         unlink(ss_program.str(), ss_shader.str());
 
-        return;
+        return true;
     }
 
     if (strcmp(name, "glUseProgram") == 0 ||
@@ -529,7 +515,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
             provide(ss.str(), call->no);
         }
 
-        return;
+        return true;
     }
 
     if (strcmp(name, "glGetUniformLocation") == 0 ||
@@ -545,7 +531,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         providef("program-", program, call->no);
 
-        return;
+        return true;
     }
 
     /* For any call that accepts 'location' as its first argument,
@@ -590,7 +576,7 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
             }
         }
 
-        return;
+        return true;
     }
 
     /* FIXME: We cut a huge swath by assuming that any unhandled
@@ -610,9 +596,16 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
 
         GLuint program = call->arg(0).toUInt();
         providef("program-", program, call->no);
-        return;
+        return true;
     }
 
+    /* No known shader-related side effects. Return false for more analysis. */
+    return false;
+}
+
+bool
+TraceAnalyzer::recordDrawingSideEffects(trace::Call *call, const char *name)
+{
     /* Handle all rendering operations, (even though only glEnd is
      * flagged as a rendering operation we treat everything from
      * glBegin through glEnd as a rendering operation). */
@@ -639,9 +632,70 @@ TraceAnalyzer::recordSideEffects(trace::Call *call)
             }
         }
 
+        return true;
+    }
+
+    /* No known drawing-related side effects. Return false for more analysis. */
+    return false;
+}
+
+void
+TraceAnalyzer::recordSideEffects(trace::Call *call)
+{
+
+    const char *name = call->name();
+
+    /* FIXME: If we encode the list of commands that are executed
+     * immediately (as opposed to those that are compiled into a
+     * display list) then we could generate a "display-list-X"
+     * resource just as we do for "texture-X" resources and only
+     * emit it in the trace if a glCallList(X) is emitted. For
+     * now, simply punt and include anything within glNewList and
+     * glEndList in the trim output. This guarantees that display
+     * lists will work, but does not trim out unused display
+     * lists. */
+    if (insideNewEndList != 0) {
+        provide("state", call->no);
+
+        /* Also, any texture bound inside a display list is
+         * conservatively considered required. */
+        if (strcmp(name, "glBindTexture") == 0) {
+            GLuint texture = call->arg(1).toUInt();
+
+            linkf("state", "texture-", texture);
+        }
+
         return;
     }
 
+    if (trimFlags & TRIM_FLAG_NO_SIDE_EFFECTS) {
+
+        if (callHasNoSideEffects(call, name)) {
+            return;
+        }
+    }
+
+    if (trimFlags & TRIM_FLAG_TEXTURES) {
+        
+        if (recordTextureSideEffects(call, name)) {
+            return;
+        }
+    }
+
+    if (trimFlags & TRIM_FLAG_SHADERS) {
+
+        if (recordShaderSideEffects(call, name)) {
+            return;
+        }
+    }
+
+    if (trimFlags & TRIM_FLAG_DRAWING) {
+
+        if (recordDrawingSideEffects(call, name)) {
+            return;
+        }
+    }
+
     /* By default, assume this call affects the state somehow. */
     resources["state"].insert(call->no);
 }
@@ -660,11 +714,13 @@ TraceAnalyzer::requireDependencies(trace::Call *call)
     consume("state");
 }
 
-TraceAnalyzer::TraceAnalyzer(): transformFeedbackActive(false),
-                               framebufferObjectActive(false),
-                               insideBeginEnd(false),
-                               insideNewEndList(0),
-                               activeTextureUnit(GL_TEXTURE0)
+TraceAnalyzer::TraceAnalyzer(TrimFlags trimFlagsOpt = -1):
+    transformFeedbackActive(false),
+    framebufferObjectActive(false),
+    insideBeginEnd(false),
+    insideNewEndList(0),
+    activeTextureUnit(GL_TEXTURE0),
+    trimFlags(trimFlagsOpt)
 {
     /* Nothing needed. */
 }
index 6619c5736e34b2cd1f193e147a23b58fdbb5e95f..ef89ad7a4eb0fc27a2f36d648b5da540fdcdb257 100644 (file)
 #include "trace_callset.hpp"
 #include "trace_parser.hpp"
 
+typedef unsigned TrimFlags;
+
+/**
+ * Trim flags
+ */
+enum {
+
+    /* Whether to trim calls that have no side effects. */
+    TRIM_FLAG_NO_SIDE_EFFECTS          = (1 << 0),
+
+    /* Whether to trim calls to setup textures that are never used. */
+    TRIM_FLAG_TEXTURES                 = (1 << 1),
+
+    /* Whether to trim calls to setup shaders that are never used. */
+    TRIM_FLAG_SHADERS                  = (1 << 2),
+
+    /* Whether to trim drawing operations outside of the desired call-set. */
+    TRIM_FLAG_DRAWING                  = (1 << 3),
+};
+
 class TraceAnalyzer {
 private:
     std::map<std::string, std::set<unsigned> > resources;
@@ -46,6 +66,7 @@ private:
     GLuint insideNewEndList;
     GLenum activeTextureUnit;
     GLuint activeProgram;
+    unsigned int trimFlags;
 
     void provide(std::string resource, trace::CallNo call_no);
     void providef(std::string resource, int resource_no, trace::CallNo call_no);
@@ -58,7 +79,13 @@ private:
     void unlinkAll(std::string resource);
 
     void stateTrackPreCall(trace::Call *call);
+
     void recordSideEffects(trace::Call *call);
+    bool callHasNoSideEffects(trace::Call *call, const char *name);
+    bool recordTextureSideEffects(trace::Call *call, const char *name);
+    bool recordShaderSideEffects(trace::Call *call, const char *name);
+    bool recordDrawingSideEffects(trace::Call *call, const char *name);
+
     void stateTrackPostCall(trace::Call *call);
 
     bool renderingHasSideEffect(void);
@@ -68,7 +95,7 @@ private:
     void requireDependencies(trace::Call *call);
 
 public:
-    TraceAnalyzer();
+    TraceAnalyzer(TrimFlags trimFlags);
     ~TraceAnalyzer();
 
     /* Analyze this call by tracking state and recording all the