From 25fe524415ade86b96e5d260134fab7de587b227 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 14 Nov 2012 20:55:31 -0800 Subject: [PATCH] trim: Add a new --trim-spec option for fine-grained trimming 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 | 66 ++++++++++++++++- cli/trace_analyzer.cpp | 162 +++++++++++++++++++++++++++-------------- cli/trace_analyzer.hpp | 29 +++++++- 3 files changed, 201 insertions(+), 56 deletions(-) diff --git a/cli/cli_trim.cpp b/cli/cli_trim.cpp index c5b0188..5625a2c 100644 --- a/cli/cli_trim.cpp +++ b/cli/cli_trim.cpp @@ -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 *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(); diff --git a/cli/trace_analyzer.cpp b/cli/trace_analyzer.cpp index 9a4553e..48153f0 100644 --- a/cli/trace_analyzer.cpp +++ b/cli/trace_analyzer.cpp @@ -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(&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. */ } diff --git a/cli/trace_analyzer.hpp b/cli/trace_analyzer.hpp index 6619c57..ef89ad7 100644 --- a/cli/trace_analyzer.hpp +++ b/cli/trace_analyzer.hpp @@ -31,6 +31,26 @@ #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 > 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 -- 2.43.0