From 90c91783fd15a72d5410104431bd35027e8bfccc Mon Sep 17 00:00:00 2001 From: unknown <94389147@qq.com> Date: Thu, 29 Mar 2018 15:15:28 +0800 Subject: [PATCH 1/5] Fix nir shaders memory leak on translator host side, this issue is heavily increased the memory usage in application created and compile shader programs frequently --- .../host/tools/emugen/ApiGen.cpp | 79 +++++++++++++++++++ src/anbox/graphics/emugl/RenderThread.cpp | 4 + 2 files changed, 83 insertions(+) diff --git a/external/android-emugl/host/tools/emugen/ApiGen.cpp b/external/android-emugl/host/tools/emugen/ApiGen.cpp index cd3d67b..e368055 100644 --- a/external/android-emugl/host/tools/emugen/ApiGen.cpp +++ b/external/android-emugl/host/tools/emugen/ApiGen.cpp @@ -753,6 +753,10 @@ int ApiGen::genDecoderHeader(const std::string &filename) fprintf(fp, "#include \"IOStream.h\" \n"); fprintf(fp, "#include \"%s_%s_context.h\"\n\n\n", m_basename.c_str(), sideString(SERVER_SIDE)); + if (strcmp(classname.c_str(), "gles2_decoder_context_t") == 0) { + fprintf(fp, "\n#include \n"); + fprintf(fp, "\n#include \n"); + } fprintf(fp, "\n#include \"emugl/common/logging.h\"\n"); for (size_t i = 0; i < m_decoderHeaders.size(); i++) { @@ -763,6 +767,17 @@ int ApiGen::genDecoderHeader(const std::string &filename) fprintf(fp, "struct %s : public %s_%s_context_t {\n\n", classname.c_str(), m_basename.c_str(), sideString(SERVER_SIDE)); fprintf(fp, "\tsize_t decode(void *buf, size_t bufsize, IOStream *stream);\n"); + if (strcmp(classname.c_str(), "gles2_decoder_context_t") == 0){ + fprintf(fp, + "\tvoid freeShader(); \n\ + \tvoid freeProgram(); \n\ + \tstd::map m_programs; \n\ + \tstd::map m_shaders; \n\ + \tstd::mutex m_lock; \n\ + "); + + + } fprintf(fp, "\n};\n\n"); fprintf(fp, "#endif // GUARD_%s\n", classname.c_str()); @@ -835,6 +850,35 @@ int ApiGen::genDecoderImpl(const std::string &filename) // helper templates fprintf(fp, "using namespace emugl;\n\n"); + // glsl shader/program free; + if (strcmp(classname.c_str(), "gles2_decoder_context_t") == 0) { + fprintf(fp, "void %s::freeShader(){\n", classname.c_str()); + fprintf(fp, + " \n\ +\tauto it = m_shaders.begin();\n\ +\tm_lock.lock();\n\ +\twhile(it != m_shaders.end()) \n\ +\t{\n\ +\t\tthis->glDeleteShader(it->first);\n\ +\t\tit++;\n\ +\t}\n\ +\tm_lock.unlock();\n\ +}\n\n"); + + fprintf(fp, "void %s::freeProgram(){\n", classname.c_str()); + fprintf(fp, + " \n\ +\tauto it = m_programs.begin(); \n\ +\tm_lock.lock();\n\ +\twhile(it != m_programs.end()) \n\ +\t{\n\ +\t\tthis->glDeleteProgram(it->first);\n\ +\t\tit++;\n\ +\t}\n\ +\tm_lock.unlock();\n\ +}\n\n"); + } + // decoder switch; fprintf(fp, "size_t %s::decode(void *buf, size_t len, IOStream *stream)\n{\n", classname.c_str()); fprintf(fp, @@ -1161,6 +1205,41 @@ int ApiGen::genDecoderImpl(const std::string &filename) } // pass; fprintf(fp, "\t\t\tSET_LASTCALL(\"%s\");\n", e->name().c_str()); + if (strcmp(m_basename.c_str(), "gles2") == 0) { + if (strcmp(e->name().c_str(), "glAttachShader") == 0){ + fprintf(fp, "\n\ + \t\t\tm_lock.lock();\n\ + m_shaders.insert({var_shader, 1});\n\ + m_lock.unlock();\n"); + } else if(strcmp(e->name().c_str(), "glDeleteProgram") == 0){ + fprintf(fp, + "\t\t\tm_lock.lock(); \n" + "\t\t\tauto pro = m_programs.find(var_program); \n" + "\t\t\tif (pro == m_programs.end()) \n" + "\t\t\t{\n" + "\t\t\t\tDEBUG(\"This should not happened!\"); \n" + "\t\t\t} else { \n" + "\t\t\t\tm_programs.erase(pro); \n" + "\t\t\t}\n" + "\t\t\tm_lock.unlock();\n"); + } else if(strcmp(e->name().c_str(), "glDeleteShader") == 0){ + fprintf(fp, + "\t\t\tm_lock.lock(); \n\ + \t\t\tauto shader = m_shaders.find(var_shader); \n\ + \t\t\tif (shader == m_shaders.end()) \n\ + \t\t\t{ \n\ + \t\t\t\tDEBUG(\"This should not happened!\"); \n\ + \t\t\t} else { \n\ + \t\t\t\tm_shaders.erase(shader); \n\ + \t\t\t} \n\ + \t\t\tm_lock.unlock(); \n"); + } else if(strcmp(e->name().c_str(), "glLinkProgram") == 0){ + fprintf(fp, " \n\ + \t\t\tm_lock.lock();\n\ + \t\t\tm_programs.insert({var_program, 1});\n\ + \t\t\tm_lock.unlock();\n"); + } + } fprintf(fp, "\t\t\tbreak;\n"); fprintf(fp, "\t\t}\n"); diff --git a/src/anbox/graphics/emugl/RenderThread.cpp b/src/anbox/graphics/emugl/RenderThread.cpp index 7e6dff7..7c8e4e6 100644 --- a/src/anbox/graphics/emugl/RenderThread.cpp +++ b/src/anbox/graphics/emugl/RenderThread.cpp @@ -84,8 +84,12 @@ intptr_t RenderThread::main() { } } while (progress); + } + threadInfo.m_gl2Dec.freeShader(); + threadInfo.m_gl2Dec.freeProgram(); + // Release references to the current thread's context/surfaces if any renderer_->bindContext(0, 0, 0); if (threadInfo.currContext || threadInfo.currDrawSurf || threadInfo.currReadSurf) From 4dd2ae389a26a64444968b00db6b7dc8f2d6c0c5 Mon Sep 17 00:00:00 2001 From: Jiancong <94389147@qq.com> Date: Tue, 3 Apr 2018 10:57:25 +0800 Subject: [PATCH 2/5] Fix the cycle-referenced caused memory leak by shared_ptr used, this issue is recognized by valgrind memcheck tool. --- external/process-cpp-minimal/include/core/posix/signal.h | 2 +- external/process-cpp-minimal/src/core/posix/signal.cpp | 8 +++----- src/anbox/cmds/session_manager.cpp | 9 +++++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/external/process-cpp-minimal/include/core/posix/signal.h b/external/process-cpp-minimal/include/core/posix/signal.h index d11724a..a647149 100644 --- a/external/process-cpp-minimal/include/core/posix/signal.h +++ b/external/process-cpp-minimal/include/core/posix/signal.h @@ -100,7 +100,7 @@ protected: * @brief Traps the specified signals for the entire process. */ CORE_POSIX_DLL_PUBLIC -std::shared_ptr trap_signals_for_process( +SignalTrap* trap_signals_for_process( std::initializer_list blocked_signals); /** diff --git a/external/process-cpp-minimal/src/core/posix/signal.cpp b/external/process-cpp-minimal/src/core/posix/signal.cpp index fd90047..4dd38cf 100644 --- a/external/process-cpp-minimal/src/core/posix/signal.cpp +++ b/external/process-cpp-minimal/src/core/posix/signal.cpp @@ -203,12 +203,10 @@ private: }; } -std::shared_ptr core::posix::trap_signals_for_process( - std::initializer_list blocked_signals) +core::posix::SignalTrap* core::posix::trap_signals_for_process( + std::initializer_list blocked_signals) { - return std::make_shared( - impl::SignalTrap::Scope::process, - blocked_signals); + return new impl::SignalTrap(impl::SignalTrap::Scope::process, blocked_signals); } std::shared_ptr core::posix::trap_signals_for_all_subsequent_threads( diff --git a/src/anbox/cmds/session_manager.cpp b/src/anbox/cmds/session_manager.cpp index 4988967..1551e81 100644 --- a/src/anbox/cmds/session_manager.cpp +++ b/src/anbox/cmds/session_manager.cpp @@ -131,7 +131,7 @@ anbox::cmds::SessionManager::SessionManager() use_system_dbus_)); action([this](const cli::Command::Context &) { - auto trap = core::posix::trap_signals_for_process( + core::posix::SignalTrap* trap = core::posix::trap_signals_for_process( {core::posix::Signal::sig_term, core::posix::Signal::sig_int}); trap->signal_raised().connect([trap](const core::posix::Signal &signal) { INFO("Signal %i received. Good night.", static_cast(signal)); @@ -139,11 +139,13 @@ anbox::cmds::SessionManager::SessionManager() }); if (standalone_ && !experimental_) { + delete trap; ERROR("Experimental features selected, but --experimental flag not set"); return EXIT_FAILURE; } if (!fs::exists("/dev/binder") || !fs::exists("/dev/ashmem")) { + delete trap; ERROR("Failed to start as either binder or ashmem kernel drivers are not loaded"); return EXIT_FAILURE; } @@ -176,8 +178,10 @@ anbox::cmds::SessionManager::SessionManager() input_manager, display_frame, single_window_); - if (!platform) + if (!platform) { + delete trap; return EXIT_FAILURE; + } auto app_db = std::make_shared(); @@ -283,6 +287,7 @@ anbox::cmds::SessionManager::SessionManager() rt->stop(); + delete trap; return EXIT_SUCCESS; }); } From 715c57564a61092a767b6adfa14a636757623af2 Mon Sep 17 00:00:00 2001 From: Jiancong <94389147@qq.com> Date: Wed, 4 Apr 2018 17:09:24 +0800 Subject: [PATCH 3/5] Remove debug info print. --- external/android-emugl/host/tools/emugen/ApiGen.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/external/android-emugl/host/tools/emugen/ApiGen.cpp b/external/android-emugl/host/tools/emugen/ApiGen.cpp index e368055..fc6c031 100644 --- a/external/android-emugl/host/tools/emugen/ApiGen.cpp +++ b/external/android-emugl/host/tools/emugen/ApiGen.cpp @@ -1215,10 +1215,8 @@ int ApiGen::genDecoderImpl(const std::string &filename) fprintf(fp, "\t\t\tm_lock.lock(); \n" "\t\t\tauto pro = m_programs.find(var_program); \n" - "\t\t\tif (pro == m_programs.end()) \n" - "\t\t\t{\n" - "\t\t\t\tDEBUG(\"This should not happened!\"); \n" - "\t\t\t} else { \n" + "\t\t\tif (pro != m_programs.end()) \n" + "\t\t\t{ \n" "\t\t\t\tm_programs.erase(pro); \n" "\t\t\t}\n" "\t\t\tm_lock.unlock();\n"); @@ -1226,10 +1224,8 @@ int ApiGen::genDecoderImpl(const std::string &filename) fprintf(fp, "\t\t\tm_lock.lock(); \n\ \t\t\tauto shader = m_shaders.find(var_shader); \n\ - \t\t\tif (shader == m_shaders.end()) \n\ + \t\t\tif (shader != m_shaders.end()) \n\ \t\t\t{ \n\ - \t\t\t\tDEBUG(\"This should not happened!\"); \n\ - \t\t\t} else { \n\ \t\t\t\tm_shaders.erase(shader); \n\ \t\t\t} \n\ \t\t\tm_lock.unlock(); \n"); From 48471b1454b6f77d4e152332f045586a56485c88 Mon Sep 17 00:00:00 2001 From: Jiancong <94389147@qq.com> Date: Thu, 5 Apr 2018 22:06:43 +0800 Subject: [PATCH 4/5] Revert previous cycling-reference commit. --- external/process-cpp-minimal/include/core/posix/signal.h | 2 +- external/process-cpp-minimal/src/core/posix/signal.cpp | 6 ++++-- src/anbox/cmds/session_manager.cpp | 6 +----- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/external/process-cpp-minimal/include/core/posix/signal.h b/external/process-cpp-minimal/include/core/posix/signal.h index a647149..d11724a 100644 --- a/external/process-cpp-minimal/include/core/posix/signal.h +++ b/external/process-cpp-minimal/include/core/posix/signal.h @@ -100,7 +100,7 @@ protected: * @brief Traps the specified signals for the entire process. */ CORE_POSIX_DLL_PUBLIC -SignalTrap* trap_signals_for_process( +std::shared_ptr trap_signals_for_process( std::initializer_list blocked_signals); /** diff --git a/external/process-cpp-minimal/src/core/posix/signal.cpp b/external/process-cpp-minimal/src/core/posix/signal.cpp index 4dd38cf..43c10e0 100644 --- a/external/process-cpp-minimal/src/core/posix/signal.cpp +++ b/external/process-cpp-minimal/src/core/posix/signal.cpp @@ -203,10 +203,12 @@ private: }; } -core::posix::SignalTrap* core::posix::trap_signals_for_process( +std::shared_ptr core::posix::trap_signals_for_process( std::initializer_list blocked_signals) { - return new impl::SignalTrap(impl::SignalTrap::Scope::process, blocked_signals); + return std::make_shared( + impl::SignalTrap::Scope::process, + blocked_signals); } std::shared_ptr core::posix::trap_signals_for_all_subsequent_threads( diff --git a/src/anbox/cmds/session_manager.cpp b/src/anbox/cmds/session_manager.cpp index 1551e81..af0bcec 100644 --- a/src/anbox/cmds/session_manager.cpp +++ b/src/anbox/cmds/session_manager.cpp @@ -131,7 +131,7 @@ anbox::cmds::SessionManager::SessionManager() use_system_dbus_)); action([this](const cli::Command::Context &) { - core::posix::SignalTrap* trap = core::posix::trap_signals_for_process( + auto trap = core::posix::trap_signals_for_process( {core::posix::Signal::sig_term, core::posix::Signal::sig_int}); trap->signal_raised().connect([trap](const core::posix::Signal &signal) { INFO("Signal %i received. Good night.", static_cast(signal)); @@ -139,13 +139,11 @@ anbox::cmds::SessionManager::SessionManager() }); if (standalone_ && !experimental_) { - delete trap; ERROR("Experimental features selected, but --experimental flag not set"); return EXIT_FAILURE; } if (!fs::exists("/dev/binder") || !fs::exists("/dev/ashmem")) { - delete trap; ERROR("Failed to start as either binder or ashmem kernel drivers are not loaded"); return EXIT_FAILURE; } @@ -179,7 +177,6 @@ anbox::cmds::SessionManager::SessionManager() display_frame, single_window_); if (!platform) { - delete trap; return EXIT_FAILURE; } @@ -287,7 +284,6 @@ anbox::cmds::SessionManager::SessionManager() rt->stop(); - delete trap; return EXIT_SUCCESS; }); } From 08defc555892d4af0e58555b6bdf4fb676e7da2f Mon Sep 17 00:00:00 2001 From: Jiancong <94389147@qq.com> Date: Sun, 6 May 2018 12:28:31 +0800 Subject: [PATCH 5/5] Remove single line bracket. --- src/anbox/cmds/session_manager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/anbox/cmds/session_manager.cpp b/src/anbox/cmds/session_manager.cpp index af0bcec..165c043 100644 --- a/src/anbox/cmds/session_manager.cpp +++ b/src/anbox/cmds/session_manager.cpp @@ -176,9 +176,8 @@ anbox::cmds::SessionManager::SessionManager() input_manager, display_frame, single_window_); - if (!platform) { + if (!platform) return EXIT_FAILURE; - } auto app_db = std::make_shared();