[PATCH 0/9] support for tracing Android Dalvik applications

[José Fonseca]

On Mon, Apr 16, 2012 at 10:10 AM, Imre Deak <imre.deak at intel.com> wrote:
> On Sun, 2012-04-15 at 13:28 +0100, José Fonseca wrote:
>> Imre,
>>
>> Thanks for these changes.
>>
>> I've already merged all which seemed riskless.
>
> Ok, great.
>
>> These need a bit more care:
>>
>> >  Android: add support for dynamically enable/disable tracing
>> >  Android: add usage instructions for tracing zygote applications
>>
>> Is it possible to do this only only on zygote and its children? I'd
>> prefer that for non-zygote processes apitrace worked in a similar
>> fashion for other platforms, i.e., without requiring any additional
>> settings.
>
> Agreed, that would cause less surprises for people already familiar with
> the current semantics. We could add a new environment variable that
> would control this, something like APITRACE_DYNAMIC_ENABLE. If it's not
> set we would have the original behavior.

What about recognizing the "zygote" process name, or something like
that, and do this automatically?

>> Also, how does this work exactly? apitrace_enabled() caches the
>> enable/disable property in a static variable. Won't this static
>> property be forked too (and its value preserved), or does this assume
>> that the zygote process never calls any GL entrypoint?
>
> You are right, if zygote calls any GL functions then the caching will
> prevent apitrace to get enabled in the forked child (the target
> process). I haven't thought about this, it only worked for me since
> zygote doesn't do such calls at the moment. I don't think it makes much
> sense for it to do, but then again there's no guarantee for this. So we
> should have something more robust.
>
>>  Perhaps we should keep track of PID, and recheck when PID changes.
>
> Yes, this would work. Since this has some overhead we could also
> consider catching the fork event, through SIGCHLD.

Given the parent never/seldom does GL calls, the overhead of getpid
should be negligible.

>> >  optionally track the GL state manually
>> >  egl: enable manual state tracking
>>
>> Have you tried the latest (and reportedly improved) Android GLES
>> emulator to see if the issues persist? If they do, then please file
>> bugs with Gogle if you haven't, as I really want to limit the amount
>> of workarounds for buggy OGL implementations as much as possible.
>
> No, I haven't checked it yet. I'll do so, thanks for pointing it out.
> While I understand why it's good to keep free of workarounds, there are
> still the following points to consider:
>
> - Traditionally it's a rather long process to have Google fix things
> through bug reports.
> - There could potentially be other non-conforming or buggy GL
> implementations (platforms) that would need the same manual tracking.

The sooner we report the better then.  I can live with workarounds
temporarily, but I honestly can't afford to care for implementations
that make no effort to be compliant with OpenGL spec or to support
tools like apitrace. Workarounds make code harder to maintain, and to
improve, so it's not a zero loss choice. I definitely don't want to
maintain a full OpenGL state tracker in Apitrace.

Also, its not really necessary to track buffer contents for GLES: the
"virtual-memory-regions" apitrace branch has a prototype of tracking
user pointers  by querying the OS' virtual memory manager. And it has
the added benefit that traces are much smaller -- as it only records
user memory when it changes, instead of recording it invariably once
per call.

> Would a compile time/runtime switch for enabling manual tracking be more
> acceptable for you? With this one could for example have an EGL wrapper
> that works in the way it does now, or one that has manual tracking on.

Compile time is OK for Android specific workarounds.

GLES workaraounds should only be enabled  for GLES context in runtime.

>> Also, before we can do this, we need to track multiple contexts
>> properly --. Essentially we need to intercept eglMakeCurrent, and
>> maintain a per-context in a TLS pointer, respect shared state, etc.
>> Otherwise we'll introduce race conditions, and risk cause crashes
>> while tracing (which is way more important than having traces that can
>> be replayed properly).
>
> Ok. I noticed that there is no explicit support for multiple contexts -
> at least for EGL -, I wasn't sure if tracing works at all in this case,
> so I didn't take this into account for the manual tracking.

Till now we were relying on the current context for most state, and
not relying on the glstate::Context for much, so multiple contexts
would have worked unless an app was mixing diffferent flavours of
GL/GLES1/GLES2

But that will change now.

> Are you
> planning to do this? If not I could take a look at it.

My attentions are focused elsewhere at this moment. So I'd appreciate
if you could take a look.

Implementation should be simple: modiffy glstate::getContext() so that
it uses a common/os_thread.hpp's thread_specific_ptr, which is
created/updated/destroyed on
eglCreateContext/MakeCurrent/eglDestroyContext. And whenever one is
not bound, just fallback to a global (as now), just to avoid crashes.

Jose