Closed
Bug 1091479
Opened 9 years ago
Closed 9 years ago
TaskTracer: SourceEvent have no record of vptr
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: gyeh, Assigned: gyeh)
References
Details
Attachments
(2 files, 4 obsolete files)
2.01 KB,
patch
|
gyeh
:
review+
|
Details | Diff | Splinter Review |
5.55 KB,
patch
|
gyeh
:
review+
|
Details | Diff | Splinter Review |
From the logging results, we found that all SourceEvents have no record of vptr which results in no symbol can be successfully extracted from libxul.so
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → gyeh
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8518794 [details] [diff] [review] Patch 1(v1): Log dispatch time when creating a source event Dispatch time is missing in source event.
Attachment #8518794 -
Flags: review?(tlee)
Comment 3•9 years ago
|
||
Comment on attachment 8518794 [details] [diff] [review] Patch 1(v1): Log dispatch time when creating a source event Review of attachment 8518794 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/GeckoTaskTracer.cpp @@ +99,5 @@ > info->mCurTaskId = newId; > > // Log a fake dispatch and start for this source event. > + LogDispatch(newId, newId, newId, aType); > + LogVirtualTablePtr(newId, newId, (int*)gFuncPtr); I think, maybe we could use a more meaningful way. For example, every type has a symbol. int Type1Event; int Type2Event; int Type3Event; static void *sEventSymbols[] = { &Type1Event, &Type2Event, &Type3Event }; CreateSourceEvent(SourceEventType aType) { ... MOZ_ASSERT(aType < (sizeof(sEventSymbols) / sizeof(sEventSymbols[0]))); LogVirtualTablePtr(newId, newId, sEventSymbols[aType]); ... } We could define all event types in a included file with a macro. The file is used to generate SourceEventType and symbols. How do you think?
Assignee | ||
Comment 4•9 years ago
|
||
Sounds great to me. :)
Assignee | ||
Comment 5•9 years ago
|
||
- Create "SourceEventTypeMap.h" - Add macro GET_SOURCE_EVENT_NAME
Attachment #8518794 -
Attachment is obsolete: true
Attachment #8518794 -
Flags: review?(tlee)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8525919 [details] [diff] [review] Patch 1(v2): Log dispatch time when creating a source event Comments are addressed. Please review the new patch.
Attachment #8525919 -
Flags: review?(tlee)
Assignee | ||
Comment 7•9 years ago
|
||
The macro is refined.
Attachment #8525919 -
Attachment is obsolete: true
Attachment #8525919 -
Flags: review?(tlee)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8526574 -
Flags: review?(tlee)
Assignee | ||
Updated•9 years ago
|
Attachment #8526573 -
Flags: review?(tlee)
Comment 9•9 years ago
|
||
Comment on attachment 8526573 [details] [diff] [review] Patch 1(v3): Log dispatch time when creating a source event Review of attachment 8526573 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/GeckoTaskTracer.cpp @@ +97,5 @@ > > + int* namePtr; > +#define SOURCE_EVENT_NAME(type) \ > + case SourceEventType::type: \ > + static int CreateSourceEvent##type; \ This may cause compiler warning. Please embrace it with brackets.
Attachment #8526573 -
Flags: review?(tlee) → review+
Updated•9 years ago
|
Attachment #8526574 -
Flags: review?(tlee) → review+
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Thinker Li [:sinker] from comment #9) > Comment on attachment 8526573 [details] [diff] [review] > Patch 1(v3): Log dispatch time when creating a source event > > Review of attachment 8526573 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: tools/profiler/GeckoTaskTracer.cpp > @@ +97,5 @@ > > > > + int* namePtr; > > +#define SOURCE_EVENT_NAME(type) \ > > + case SourceEventType::type: \ > > + static int CreateSourceEvent##type; \ > > This may cause compiler warning. Please embrace it with brackets. Thanks. I'll update my final patch.
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8526573 -
Attachment is obsolete: true
Attachment #8526623 -
Flags: review+
Assignee | ||
Comment 12•9 years ago
|
||
Attachment #8526574 -
Attachment is obsolete: true
Attachment #8526625 -
Flags: review+
Assignee | ||
Comment 13•9 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=50c565559640
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/937387b0d0bf https://hg.mozilla.org/integration/mozilla-inbound/rev/c178550109f8
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/937387b0d0bf https://hg.mozilla.org/mozilla-central/rev/c178550109f8
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•