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

[Imre Deak]

On Tue, 2012-04-17 at 22:39 +0100, José Fonseca wrote:
> 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?

Ok, we can do that.

> >> 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.

Ok.

> >> >  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.

Sounds good, I will check it out. But I guess it's ok with you to have
the less optimal way of coping for the time being.

> > 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.

Ok.

> >> 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.

Ok, will take a look at these and resend the patchset when I have
something working.

--Imre