<div dir="ltr">On Mon, May 13, 2013 at 10:34 AM, Alexander Monakov <span dir="ltr"><<a href="mailto:amonakov@ispras.ru" target="_blank">amonakov@ispras.ru</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hello,<br>
<div class="im"><br>
On Wed, 8 May 2013, José Fonseca wrote:<br>
<br>
> - please confirm that Android was not busted with my changes (as I only<br>
> tested on Linux)<br>
<br>
</div>It needs the following patch to unbreak the build:<br>
<br>
diff --git a/common/trace_backtrace.cpp b/common/trace_backtrace.cpp<br>
index ef19aa9..d0bdd5a 100644<br>
--- a/common/trace_backtrace.cpp<br>
+++ b/common/trace_backtrace.cpp<br>
@@ -243,7 +243,7 @@ public:<br>
<div class="im">         while (*rawBacktrace_it != '\0') {<br>
</div>             RawStackFrame stackFrame;<br>
             // TODO: Keep a cache of stack frames<br>
-            stackFrame->id = nextFrameId++;<br>
+            stackFrame.id = nextFrameId++;<br>
<div class="im">             /* skip leading space */<br>
</div>             while (*rawBacktrace_it == ' ') {<br>
                 rawBacktrace_it++;<br>
<div class="im"><br>
<br></div></blockquote><div><br></div><div style>Applied thanks.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">
> - please make default not capture backtraces on linux (I'm fine leaving<br>
> backtraces on by default on Android if nobody else opposes) -- this is<br>
> temporarily until backtrace capture is fast enough that the overhead is<br>
> negligble (which I think it should be possible)<br>
<br>
</div>We (both Eugene and I) don't quite understand what happened here.  You have<br>
eliminated usage of 'backtrace_is_needed', removing the ability to select the<br>
subset of "backtraced" calls.  Was that intentional?  This was not raised in<br>
preceding review, and you did not prune the resulting dead code.<br></blockquote><div><br></div><div style>No, it was not intentional. I removed by mistake, and somehow thought that it was only used in Android. I've re-added backtrace_is_needed() now.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Non-zero overhead from backtrace collection was the reason Eugene implemented<br>
that subsetting in the first place.<br></blockquote><div> <br></div></div>OK. It seems good enough for now. I'll merge to master shortly after sanity check on windows/macosx.</div><div class="gmail_extra"><br></div><div class="gmail_extra">
<br></div><div class="gmail_extra" style>BTW, another issue is that the "offset" values on linux are not really the offset from start of shared object, but the absolute address. Anyway, I think that using glibc backtrace for this is unsustainable long term, as it requires executables to be built with -rdynamic, and shared objects not to be built with -fvisibility=hidden, the exact opposite of common practice these days.  I don't know if there is a better alternative for runtime symbol resolution, otherwise I think we'll need to do offline symbol resolution on linux by default.</div>
<div class="gmail_extra" style><br></div><div class="gmail_extra" style><br></div><div class="gmail_extra" style>One last request -- could you make it so that when one selects a call on qapitrace, the default lower pane is the "Details View" (instead of the "Backtrace").</div>
<div class="gmail_extra" style><br></div><div class="gmail_extra" style><br></div><div class="gmail_extra" style>Jose</div></div>