From 540080421d4f6da04b855e4b3096156ee66aca7e Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 28 Aug 2018 09:37:00 -0400 Subject: [PATCH] Fix review comments for replay_archive. --- cmd/gapir/cc/main.cpp | 219 +++++++++++++++++++--------------- core/archive/archive.go | 2 +- gapir/cc/replay_archive.cpp | 6 +- gapis/api/vulkan/replay.go | 18 ++- gapis/replay/export_replay.go | 2 - 5 files changed, 146 insertions(+), 101 deletions(-) diff --git a/cmd/gapir/cc/main.cpp b/cmd/gapir/cc/main.cpp index 4b7c60573b..e08956e27f 100644 --- a/cmd/gapir/cc/main.cpp +++ b/cmd/gapir/cc/main.cpp @@ -201,8 +201,9 @@ void android_main(struct android_app* app) { #else // TARGET_OS == GAPID_OS_ANDROID -// Main function for PC -int main(int argc, const char* argv[]) { +namespace { + +struct Options { int logLevel = LOG_LEVEL; const char* logPath = "logs/gapir.log"; @@ -212,120 +213,124 @@ int main(int argc, const char* argv[]) { const char* authTokenFile = nullptr; int idleTimeoutSec = 0; const char* replayArchive = nullptr; + bool version = false; - for (int i = 1; i < argc; i++) { - if (strcmp(argv[i], "--replay-archive") == 0) { - replayArchive = argv[++i]; - } else if (strcmp(argv[i], "--auth-token-file") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --auth-token-file "); - } - authTokenFile = argv[++i]; - } else if (strcmp(argv[i], "--cache") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --cache "); - } - cachePath = argv[++i]; - } else if (strcmp(argv[i], "--port") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --port "); - } - portArgStr = argv[++i]; - } else if (strcmp(argv[i], "--log-level") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --log-level "); - } - switch (argv[++i][0]) { - case 'F': - logLevel = LOG_LEVEL_FATAL; - break; - case 'E': - logLevel = LOG_LEVEL_ERROR; - break; - case 'W': - logLevel = LOG_LEVEL_WARNING; - break; - case 'I': - logLevel = LOG_LEVEL_INFO; - break; - case 'D': - logLevel = LOG_LEVEL_DEBUG; - break; - case 'V': - logLevel = LOG_LEVEL_VERBOSE; - break; - default: + static Options Parse(int argc, const char* argv[]) { + Options opts; + + for (int i = 1; i < argc; i++) { + if (strcmp(argv[i], "--replay-archive") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --replay-archive "); + } + opts.replayArchive = argv[++i]; + } else if (strcmp(argv[i], "--auth-token-file") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --auth-token-file "); + } + opts.authTokenFile = argv[++i]; + } else if (strcmp(argv[i], "--cache") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --cache "); + } + opts.cachePath = argv[++i]; + } else if (strcmp(argv[i], "--port") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --port "); + } + opts.portArgStr = argv[++i]; + } else if (strcmp(argv[i], "--log-level") == 0) { + if (i + 1 >= argc) { GAPID_FATAL("Usage: --log-level "); + } + switch (argv[++i][0]) { + case 'F': + opts.logLevel = LOG_LEVEL_FATAL; + break; + case 'E': + opts.logLevel = LOG_LEVEL_ERROR; + break; + case 'W': + opts.logLevel = LOG_LEVEL_WARNING; + break; + case 'I': + opts.logLevel = LOG_LEVEL_INFO; + break; + case 'D': + opts.logLevel = LOG_LEVEL_DEBUG; + break; + case 'V': + opts.logLevel = LOG_LEVEL_VERBOSE; + break; + default: + GAPID_FATAL("Usage: --log-level "); + } + } else if (strcmp(argv[i], "--log") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --log "); + } + opts.logPath = argv[++i]; + } else if (strcmp(argv[i], "--idle-timeout-sec") == 0) { + if (i + 1 >= argc) { + GAPID_FATAL("Usage: --idle-timeout-sec "); + } + opts.idleTimeoutSec = atoi(argv[++i]); + } else if (strcmp(argv[i], "--wait-for-debugger") == 0) { + opts.wait_for_debugger = true; + } else if (strcmp(argv[i], "--version") == 0) { + opts.version = true; + } else { + GAPID_FATAL("Unknown argument: %s", argv[i]); } - } else if (strcmp(argv[i], "--log") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --log "); - } - logPath = argv[++i]; - } else if (strcmp(argv[i], "--idle-timeout-sec") == 0) { - if (i + 1 >= argc) { - GAPID_FATAL("Usage: --idle-timeout-sec "); - } - idleTimeoutSec = atoi(argv[++i]); - } else if (strcmp(argv[i], "--wait-for-debugger") == 0) { - wait_for_debugger = true; - } else if (strcmp(argv[i], "--version") == 0) { - printf("GAPIR version " GAPID_VERSION_AND_BUILD "\n"); - return 0; - } else { - GAPID_FATAL("Unknown argument: %s", argv[i]); } + return opts; } +}; -#if TARGET_OS == GAPID_OS_LINUX - // Ignore SIGPIPE so we can log after gapis closes. - signal(SIGPIPE, SIG_IGN); -#endif - - if (wait_for_debugger) { - GAPID_INFO("Waiting for debugger to attach"); - core::Debugger::waitForAttach(); - } +} // namespace - if (replayArchive) { - core::CrashHandler crashHandler; - GAPID_LOGGER_INIT(logLevel, "gapir", logPath); - MemoryManager memoryManager(memorySizes); - std::string payloadPath = std::string(replayArchive) + "/payload.bin"; - gapir::ReplayArchive conn(payloadPath); - std::unique_ptr resourceProvider = - ResourceDiskCache::create(nullptr, replayArchive); - std::unique_ptr context = Context::create( - &conn, crashHandler, resourceProvider.get(), &memoryManager); - - GAPID_INFO("Replay started"); - bool ok = context->interpret(); - GAPID_INFO("Replay %s", ok ? "finished successfully" : "failed"); - - return ok ? EXIT_SUCCESS : EXIT_FAILURE; - } +static int replayArchive(Options opts) { + // The directory consists an archive(resources.{index,data}) and payload.bin. + core::CrashHandler crashHandler; + GAPID_LOGGER_INIT(opts.logLevel, "gapir", opts.logPath); + MemoryManager memoryManager(memorySizes); + std::string payloadPath = std::string(opts.replayArchive) + "/payload.bin"; + gapir::ReplayArchive conn(payloadPath); + std::unique_ptr resourceProvider = + ResourceDiskCache::create(nullptr, opts.replayArchive); + std::unique_ptr context = Context::create( + &conn, crashHandler, resourceProvider.get(), &memoryManager); + + GAPID_INFO("Replay started"); + bool ok = context->interpret(); + GAPID_INFO("Replay %s", ok ? "finished successfully" : "failed"); + + return ok ? EXIT_SUCCESS : EXIT_FAILURE; +} +static int startServer(Options opts) { core::CrashHandler crashHandler; - GAPID_LOGGER_INIT(logLevel, "gapir", logPath); + GAPID_LOGGER_INIT(opts.logLevel, "gapir", opts.logPath); // Read the auth-token. // Note: This must come before the socket is created as the auth token // file is deleted by GAPIS as soon as the port is written to stdout. std::vector authToken; - if (authTokenFile != nullptr) { - FILE* file = fopen(authTokenFile, "rb"); + if (opts.authTokenFile != nullptr) { + FILE* file = fopen(opts.authTokenFile, "rb"); if (file == nullptr) { - GAPID_FATAL("Unable to open auth-token file: %s", authTokenFile); + GAPID_FATAL("Unable to open auth-token file: %s", opts.authTokenFile); } if (fseek(file, 0, SEEK_END) != 0) { - GAPID_FATAL("Unable to get length of auth-token file: %s", authTokenFile); + GAPID_FATAL("Unable to get length of auth-token file: %s", + opts.authTokenFile); } size_t size = ftell(file); fseek(file, 0, SEEK_SET); authToken.resize(size + 1, 0); if (fread(&authToken[0], 1, size, file) != size) { - GAPID_FATAL("Unable to read auth-token file: %s", authTokenFile); + GAPID_FATAL("Unable to read auth-token file: %s", opts.authTokenFile); } fclose(file); } @@ -334,7 +339,7 @@ int main(int argc, const char* argv[]) { // If the user does not assign a port to use, get a free TCP port from OS. const char local_host_name[] = "127.0.0.1"; - std::string portStr(portArgStr); + std::string portStr(opts.portArgStr); if (portStr == "0") { uint32_t port = SocketConnection::getFreePort(local_host_name); if (port == 0) { @@ -349,7 +354,8 @@ int main(int argc, const char* argv[]) { std::mutex lock; std::unique_ptr server = Setup(uri.c_str(), (authToken.size() > 0) ? authToken.data() : nullptr, - cachePath, idleTimeoutSec, &crashHandler, &memoryManager, &lock); + opts.cachePath, opts.idleTimeoutSec, &crashHandler, &memoryManager, + &lock); // The following message is parsed by launchers to detect the selected port. // DO NOT CHANGE! printf("Bound on port '%s'\n", portStr.c_str()); @@ -361,4 +367,27 @@ int main(int argc, const char* argv[]) { return EXIT_SUCCESS; } +// Main function for PC +int main(int argc, const char* argv[]) { + Options opts = Options::Parse(argc, argv); + +#if TARGET_OS == GAPID_OS_LINUX + // Ignore SIGPIPE so we can log after gapis closes. + signal(SIGPIPE, SIG_IGN); +#endif + + if (opts.wait_for_debugger) { + GAPID_INFO("Waiting for debugger to attach"); + core::Debugger::waitForAttach(); + } + if (opts.version) { + printf("GAPIR version " GAPID_VERSION_AND_BUILD "\n"); + return EXIT_SUCCESS; + } else if (opts.replayArchive) { + return replayArchive(opts); + } else { + return startServer(opts); + } +} + #endif // TARGET_OS == GAPID_OS_ANDROID diff --git a/core/archive/archive.go b/core/archive/archive.go index 70e101239f..c3990d9ba3 100644 --- a/core/archive/archive.go +++ b/core/archive/archive.go @@ -31,7 +31,7 @@ func New(name string) Archive { return C.archive_create(cstr) } -// Dispose flush and close the underlying archive. +// Dispose flushes and closes the underlying archive. func (a Archive) Dispose() { C.archive_destroy(a) } diff --git a/gapir/cc/replay_archive.cpp b/gapir/cc/replay_archive.cpp index 13b3ea248f..11e854a271 100644 --- a/gapir/cc/replay_archive.cpp +++ b/gapir/cc/replay_archive.cpp @@ -15,6 +15,7 @@ */ #include "replay_archive.h" +#include "core/cc/log.h" #include #include @@ -24,20 +25,21 @@ namespace gapir { -std::unique_ptr ReplayArchive::getPayload() { +std::unique_ptr ReplayArchive::getPayload() { std::fstream input(mFileprefix, std::ios::in | std::ios::binary); std::unique_ptr payload(new replay_service::Payload); payload->ParseFromIstream(&input); return std::unique_ptr(new Payload(std::move(payload))); } -std::unique_ptr ReplayArchive::getResources( +std::unique_ptr ReplayArchive::getResources( std::unique_ptr req) { return nullptr; } bool ReplayArchive::sendReplayFinished() { return true; } bool ReplayArchive::sendCrashDump(const std::string& filepath, const void* crash_data, uint32_t crash_size) { + GAPID_INFO("Crash dump saved at: %s", filepath.c_str()); return true; } bool ReplayArchive::sendPostData(std::unique_ptr posts) { return true; } diff --git a/gapis/api/vulkan/replay.go b/gapis/api/vulkan/replay.go index db9c25ab08..22a650eb92 100644 --- a/gapis/api/vulkan/replay.go +++ b/gapis/api/vulkan/replay.go @@ -500,6 +500,10 @@ type issuesRequest struct { displayToSurface bool } +// exportReplayRequest requests full trace to be produced. +type exportReplayRequest struct { +} + func (a API) Replay( ctx context.Context, intent replay.Intent, @@ -590,6 +594,18 @@ func (a API) Replay( doDisplayToSurface = true } + case exportReplayRequest: + // TODO: Implement a transform specifically for export replay + if issues == nil { + n, err := expandCommands(false) + if err != nil { + return err + } + issues = newFindIssues(ctx, capture, n) + } + issues.reportTo(rr.Result) + optimize = false + case framebufferRequest: cfg := cfg.(drawConfig) @@ -784,5 +800,5 @@ func (a API) QueryIssues( // ExportReplayRequest returns request type for standalone replay. func (a API) ExportReplayRequest() replay.Request { - return issuesRequest{} + return exportReplayRequest{} } diff --git a/gapis/replay/export_replay.go b/gapis/replay/export_replay.go index bfc56869f4..8eea452a09 100644 --- a/gapis/replay/export_replay.go +++ b/gapis/replay/export_replay.go @@ -64,8 +64,6 @@ func ExportReplay(ctx context.Context, pCapture *path.Capture, pDevice *path.Dev return log.Errf(ctx, nil, "Unknown device %v", pDevice.ID.ID()) } - // executeCounter.Increment() - ctx = log.V{ "capture": pCapture.ID.ID(), "device": d.Instance().GetName(),