Skip to content

Commit

Permalink
deviceinfo: Do the device instance querying on a separate thread.
Browse files Browse the repository at this point in the history 
The queries for GL information was corrupting the currently bound EGL context. Instead of jumping through hoops to store and restore the context, just do the query on a separate throw-away thread.

Also clean up the messy logic in `Spy::get()` which was unnecessarily complicated and can now be simplified due to the explicit installation of the interceptor.

Fixes some traces where contexts were corrupted when traced.
  • Loading branch information
@ben-clayton
ben-clayton committed Oct 25, 2017
1 parent d649554 commit 9c2f3bc
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 59 deletions.
4 changes: 3 additions & 1 deletion core/os/device/deviceinfo/cc/android/jni.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
extern "C" {

jbyteArray Java_com_google_android_gapid_DeviceInfoService_getDeviceInfo(JNIEnv* env) {
auto device_instance = get_device_instance(env);
JavaVM* vm = nullptr;
env->GetJavaVM(&vm);
auto device_instance = get_device_instance(vm);
auto out = env->NewByteArray(device_instance.size);
auto data = reinterpret_cast<jbyte*>(device_instance.data);
env->SetByteArrayRegion(out, 0, device_instance.size, data);
Expand Down
28 changes: 27 additions & 1 deletion core/os/device/deviceinfo/cc/android/query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,29 @@ bool createContext(void* platform_data) {
return false; \
}

JNIEnv* env = reinterpret_cast<JNIEnv*>(platform_data);
JavaVM* vm = reinterpret_cast<JavaVM*>(platform_data);
JNIEnv* env = nullptr;
auto res = vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
bool shouldDetach = false;
switch (res) {
case JNI_OK:
break;
case JNI_EDETACHED:
res = vm->AttachCurrentThread(&env, nullptr);
if (res != 0) {
snprintf(gContext.mError, sizeof(gContext.mError),
"Failed to attach thread to JavaVM. (%d)", res);
destroyContext();
return false;
}
shouldDetach = true;
break;
default:
snprintf(gContext.mError, sizeof(gContext.mError),
"Failed to get Java env. (%d)", res);
destroyContext();
return false;
}

Class build(env, "android/os/Build");
CHECK(build.get_field("SUPPORTED_ABIS", gContext.mSupportedABIs));
Expand All @@ -285,6 +307,10 @@ bool createContext(void* platform_data) {
CHECK(version.get_field("RELEASE", gContext.mOSName));
CHECK(version.get_field("SDK_INT", gContext.mOSVersion));

if (shouldDetach) {
vm->DetachCurrentThread();
}

#undef CHECK

if (gContext.mSupportedABIs.size() > 0) {
Expand Down
25 changes: 19 additions & 6 deletions core/os/device/deviceinfo/cc/query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include <city.h>

#include <thread>

namespace {

inline bool isLittleEndian() {
Expand All @@ -41,16 +43,12 @@ device::DataTypeLayout* new_dt_layout() {
return out;
}

} // anonymous namespace

namespace query {

device::Instance* getDeviceInstance(void* platform_data) {
void buildDeviceInstance(void* platform_data, device::Instance** out) {
using namespace device;
using namespace google::protobuf::io;

if (!query::createContext(platform_data)) {
return nullptr;
return;
}

// OS
Expand Down Expand Up @@ -126,6 +124,21 @@ device::Instance* getDeviceInstance(void* platform_data) {

query::destroyContext();

*out = instance;
}

} // anonymous namespace

namespace query {

device::Instance* getDeviceInstance(void* platform_data) {
device::Instance* instance = nullptr;

// buildDeviceInstance on a seperate thread to avoid EGL screwing with the
// currently bound context.
std::thread thread(buildDeviceInstance, platform_data, &instance);
thread.join();

return instance;
}

Expand Down
77 changes: 26 additions & 51 deletions gapii/cc/spy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,8 @@ jint JNI_OnLoad(JavaVM *vm, void *reserved) {
gapii::Spy::get(); // Construct the spy.
return JNI_VERSION_1_6;
}
void* queryPlatformData() {
JNIEnv* env = nullptr;

auto res = gJavaVM->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
switch (res) {
case JNI_OK:
break;
case JNI_EDETACHED:
res = gJavaVM->AttachCurrentThread(&env, nullptr);
if (res != 0) {
GAPID_FATAL("Failed to attach thread to JavaVM. (%d)", res);
}
break;
default:
GAPID_FATAL("Failed to get Java env. (%d)", res);
}
GAPID_INFO("queryPlatformData() env = %p", env);
return env;
}

void* queryPlatformData() { return gJavaVM; }
#else // TARGET_OS == GAPID_OS_ANDROID
void* queryPlatformData() { return nullptr; }
#endif // TARGET_OS == GAPID_OS_ANDROID
Expand Down Expand Up @@ -137,25 +120,15 @@ thread_local gapii::CallObserver* gContext = nullptr;
namespace gapii {

Spy* Spy::get() {
bool init;
{
std::lock_guard<std::recursive_mutex> lock(gMutex);
init = !gSpy;
if (init) {
GAPID_INFO("Constructing spy...");
gSpy.reset(new Spy());
GAPID_INFO("Registering spy symbols...");
for (int i = 0; kGLESExports[i].mName != NULL; ++i) {
gSpy->RegisterSymbol(kGLESExports[i].mName, kGLESExports[i].mFunc);
}
std::lock_guard<std::recursive_mutex> lock(gMutex);
if (!gSpy) {
GAPID_INFO("Constructing spy...");
gSpy.reset(new Spy());
GAPID_INFO("Registering spy symbols...");
for (int i = 0; kGLESExports[i].mName != NULL; ++i) {
gSpy->RegisterSymbol(kGLESExports[i].mName, kGLESExports[i].mFunc);
}
}
if (init) {
auto s = gSpy.get();
s->enter("writeHeader", 0);
s->writeHeader();
s->exit();
}
return gSpy.get();
}

Expand Down Expand Up @@ -204,6 +177,20 @@ Spy::Spy()
mSuspendCaptureFrames.store((header.mFlags & ConnectionHeader::FLAG_DEFER_START)?
kSuspendIndefinitely: mSuspendCaptureFrames.load());

set_valid_apis(header.mAPIs);
GAPID_ERROR("APIS %08x", header.mAPIs);
GAPID_INFO("GAPII connection established. Settings:");
GAPID_INFO("Observe framebuffer every %d frames", mObserveFrameFrequency);
GAPID_INFO("Observe framebuffer every %d draws", mObserveDrawFrequency);
GAPID_INFO("Disable precompiled shaders: %s", mDisablePrecompiledShaders ? "true" : "false");

mEncoder = gapii::PackEncoder::create(mConnection);

// writeHeader needs to come before the installer is created as the
// deviceinfo queries want to call into EGL / GL commands which will be
// patched.
writeHeader();

#if TARGET_OS == GAPID_OS_ANDROID
if (strlen(header.mLibInterceptorPath) > 0) {
gInstaller = std::unique_ptr<Installer>(new Installer(header.mLibInterceptorPath));
Expand All @@ -214,15 +201,6 @@ Spy::Spy()
}
#endif // TARGET_OS == GAPID_OS_ANDROID

set_valid_apis(header.mAPIs);
GAPID_ERROR("APIS %08x", header.mAPIs);
GAPID_INFO("GAPII connection established. Settings:");
GAPID_INFO("Observe framebuffer every %d frames", mObserveFrameFrequency);
GAPID_INFO("Observe framebuffer every %d draws", mObserveDrawFrequency);
GAPID_INFO("Disable precompiled shaders: %s", mDisablePrecompiledShaders ? "true" : "false");

mEncoder = gapii::PackEncoder::create(mConnection);

auto context = enter("init", 0);
GlesSpy::init();
VulkanSpy::init();
Expand All @@ -245,15 +223,11 @@ Spy::Spy()
}

void Spy::writeHeader() {
if (!query::createContext(queryPlatformData())) {
GAPID_ERROR("query::createContext() errored: %s", query::contextError());
}
capture::Header file_header;
file_header.set_version(CurrentCaptureVersion);
file_header.set_allocated_device(query::getDeviceInstance(queryPlatformData()));
file_header.set_allocated_abi(query::currentABI());
mEncoder->object(&file_header);
query::destroyContext();
}

void Spy::resolveImports() {
Expand Down Expand Up @@ -310,9 +284,10 @@ EGLContext Spy::eglCreateContext(CallObserver* observer, EGLDisplay display, EGL
auto res = GlesSpy::eglCreateContext(observer, display, config, share_context, attrib_vector.data());

// NB: The getters modify the std::map, so this log must be last.
GAPID_INFO("eglCreateContext requested: GL %i.%i, profile 0x%x, flags 0x%x",
GAPID_INFO("eglCreateContext requested: GL %i.%i, profile 0x%x, flags 0x%x -> %p",
attribs[EGL_CONTEXT_MAJOR_VERSION_KHR], attribs[EGL_CONTEXT_MINOR_VERSION_KHR],
attribs[EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR], attribs[EGL_CONTEXT_FLAGS_KHR]);
attribs[EGL_CONTEXT_OPENGL_PROFILE_MASK_KHR], attribs[EGL_CONTEXT_FLAGS_KHR],
res);
return res;
}

Expand Down

0 comments on commit 9c2f3bc

Please sign in to comment.