1091479 - TaskTracer: SourceEvent have no record of vptr

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Gina Yeh [:gyeh] [:ginayeh]]

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

Comment 1

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v1): Log dispatch time when creating a source event

Comment 2

[Gina Yeh [:gyeh] [:ginayeh]]

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.

Comment 3

[Thinker Li [:sinker]]

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?

Comment 4

[Gina Yeh [:gyeh] [:ginayeh]]

Sounds great to me. :)

Comment 5

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v2): Log dispatch time when creating a source event

- Create "SourceEventTypeMap.h"
- Add macro GET_SOURCE_EVENT_NAME

Comment 6

[Gina Yeh [:gyeh] [:ginayeh]]

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.

Comment 7

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v3): Log dispatch time when creating a source event

The macro is refined.

Comment 8

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 2(v1): Rename enum members

Comment 9

[Thinker Li [:sinker]]

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.

Comment 10

[Gina Yeh [:gyeh] [:ginayeh]]

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

Comment 11

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1: Log dispatch time when creating a source event, r=sinker.

Comment 12

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 2: Rename enum members, r=sinker.

Comment 13

[Gina Yeh [:gyeh] [:ginayeh]]

try: https://tbpl.mozilla.org/?tree=Try&rev=50c565559640

Comment 14

[Gina Yeh [:gyeh] [:ginayeh]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/937387b0d0bf
https://hg.mozilla.org/integration/mozilla-inbound/rev/c178550109f8

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/937387b0d0bf
https://hg.mozilla.org/mozilla-central/rev/c178550109f8