Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Profile Parse Error #675

Closed
gregtatum opened this issue Dec 13, 2017 · 8 comments
Closed

JSON Profile Parse Error #675

gregtatum opened this issue Dec 13, 2017 · 8 comments
Assignees
Labels
bug Very important to fix, typically this means that the tool is broken or lying

Comments

@gregtatum
Copy link
Member

I've seen this one in the wild, and caught it. This one was on a wasm recording.

error-profile.json.zip

@gregtatum gregtatum added the bug Very important to fix, typically this means that the tool is broken or lying label Dec 13, 2017
@gregtatum
Copy link
Member Author

It's line 232810.

...
    "data": [
     
,
     [
      82,
      10553395.239479,
      {"type": "GCSlice", "startTime": 10553395.239479, "endTime": 10553398.511756001, "timings": {
...

@julienw
Copy link
Contributor

julienw commented Dec 14, 2017

Current educated guess: this comes from https://searchfox.org/mozilla-central/rev/b0098afaeaad24a6752aa418ca3e86eade5fab17/tools/profiler/core/ThreadInfo.cpp#230-233

      if (aSavedStreamedMarkers) {
        MOZ_ASSERT(aSinceTime == 0);
        aWriter.Splice(aSavedStreamedMarkers);
      }
      // then calls StreamMarkersToJSON

aSavedStreamedMarkers itself comes from https://searchfox.org/mozilla-central/rev/b0098afaeaad24a6752aa418ca3e86eade5fab17/tools/profiler/core/ThreadInfo.cpp#271-280
in ThreadInfo::FlushSamplesAndMarkers:

    SpliceableChunkedJSONWriter b;
    b.StartBareList();
    {
      aBuffer.StreamMarkersToJSON(b, mThreadId, aProcessStartTime,
                                  /* aSinceTime = */ 0, *mUniqueStacks);
    }
    b.EndBareList();
    mSavedStreamedMarkers = b.WriteFunc()->CopyData();

My guess is that we have no markers and we end up with an empty mSavedStreamedMarkers. So aSavedStreamedMarkers later is not null, but empty. So we end up writing it (see splice implementation, sets "mNeedsComma" to true) even if it's empty, and then adding a comma, which looks a lot like we see in the json above.

So 2 possible ways to fix this:

  1. in FlushSamplesAndMarkers, set mSavedStreamedMarkers to nullptr if the list is empty.
  2. in StreamSamplesAndMarkers, check that aSavedStreamedMarkers isn't empty. But I think we should trim it first so that we're sure it's not just empty spaces.

So I'd rather go with 1 (and likely fix the similar code for samples above).

@julienw
Copy link
Contributor

julienw commented Dec 14, 2017

Although I don't know how to check if the list is empty for this thread id :) Maybe 2 is easier then.

@julienw
Copy link
Contributor

julienw commented Dec 14, 2017

I reproduce 100% with the following STR:

  1. Configure the profile to capture workers (add ,Worker in the list of threads to profile)
  2. Start the profiler.
  3. Open the debugger
  4. Close the devtools
  5. Capture

I think this confirms my guess:

  • if we don't capture workers, we don't have the issue
  • if we don't close the devtools, we don't have the issue

=> so my guess is that one of the debugger's worker doesn't add any marker.

I tried to reproduce with a simpler example https://everlong.org/mozilla/testcase-worker/ but this doesn't reproduce much (I think I reproduced 1 out of 10).

@julienw julienw self-assigned this Jan 4, 2018
@julienw
Copy link
Contributor

julienw commented Jan 4, 2018

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1428076 because this will need a gecko patch. Closing here.

@julienw julienw closed this as completed Jan 4, 2018
@julienw
Copy link
Contributor

julienw commented Jan 12, 2018

reopning for tracking

@julienw julienw reopened this Jan 12, 2018
@julienw julienw added this to In Sprint in Sprint Planning 2018 Jan 12, 2018
@julienw
Copy link
Contributor

julienw commented Jan 25, 2018

Autolanding in progress in https://bugzilla.mozilla.org/show_bug.cgi?id=1428076

@julienw julienw moved this from In Sprint to In Progress in Sprint Planning 2018 Jan 25, 2018
@julienw
Copy link
Contributor

julienw commented Jan 26, 2018

done !

@julienw julienw closed this as completed Jan 26, 2018
Sprint Planning 2018 automation moved this from In Progress to Done Jan 26, 2018
@gregtatum gregtatum removed this from Done in Sprint Planning 2018 Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Very important to fix, typically this means that the tool is broken or lying
Projects
None yet
Development

No branches or pull requests

2 participants