1145333 - Skip nativeStackAddrs that conflict with pseudoStackAddrs and jsStackAddrs in mergeStacksIntoProfile

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Kannan Vijayan [:djvj]]

Sometimes, in mergeStacksIntoProfile in TableTicker.cpp, we can get native stack addrs that are the same as a pseudstack addr, or a js stack addr.

Currently, we assert that this does not happen, but it can happen in rare cases.  We should handle this case instead of just asserting it, and give preference to pseudostack or jsstack interpretations over the nativestack interpretation.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: skip-conflicting-native-stackaddrs.patch

Comment 2

[Kannan Vijayan [:djvj]]

Attachment: skip-conflicting-native-stackaddrs.patch

Updated patch with comment.

Comment 4

[Julian Seward [:jseward]]

Even with that in place, I've seen occasional assertion failures further down,
here:

    // If we reach here, there must be a native stack entry and it must be the
    // greatest entry.
    MOZ_ASSERT(nativeStackAddr);  <----------------- HERE
    MOZ_ASSERT(nativeIndex >= 0);
    aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
    nativeIndex--;

I suspect it is possible to get to this point with nativeStackAddr,
jsStackAddr, pseudoStackAddr all being zero.  Does that sound
plausible?  If we have all 3 being zero, then this doesn't apply

    // Check to see if pseudoStack frame is top-most.
    if (pseudoStackAddr > jsStackAddr && pseudoStackAddr > nativeStackAddr) {

and neither does this

    // Check to see if JS jit stack frame is top-most
    if (jsStackAddr > nativeStackAddr) {

so we wind up failing here:

    MOZ_ASSERT(nativeStackAddr);

It's very difficult to reproduce, though -- I've only seen it about
three or four times.  So I can't yet confirm the all-3-values-zero
hypothesis.

Comment 5

[Julian Seward [:jseward]]

(In reply to Julian Seward [:jseward] from comment #4)
> I suspect it is possible to get to this point with nativeStackAddr,
> jsStackAddr, pseudoStackAddr all being zero.

Verified:

    // If we reach here, there must be a native stack entry and it must be the
    // greatest entry.
if (!nativeStackAddr)
fprintf(stderr, "XXXXXXXXXXXXXXXXXXXXX %p %p %p\n", nativeStackAddr, jsStackAddr, pseudoStackAddr);
    MOZ_ASSERT(nativeStackAddr);

produces

XXXXXXXXXXXXXXXXXXXXX (nil) (nil) (nil)
Assertion failure: nativeStackAddr, at /home/sewardj/MOZ/MC-PROF/tools/profiler/TableTicker.cpp:626

Comment 6

[Julian Seward [:jseward]]

Any opinions about this as a fix?

     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
-    MOZ_ASSERT(nativeStackAddr);
-    MOZ_ASSERT(nativeIndex >= 0);
-    aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
-    nativeIndex--;
+    if (nativeStackAddr) {
+      MOZ_ASSERT(nativeIndex >= 0);
+      aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
+    }
+    if (nativeIndex >= 0) {
+      nativeIndex--;
+    }

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3abdf236d4f2