RESOLVED WONTFIX 70761
[chromium] Add category and TraceValue support to TraceEvent
https://bugs.webkit.org/show_bug.cgi?id=70761
Summary [chromium] Add category and TraceValue support to TraceEvent
Nat Duca
Reported 2011-10-24 14:00:17 PDT
The current TraceEvent plumbing does not support trace categories (~= WebCore log channels), or efficient tracing of events with arguments. We should fix this.
Attachments
Vincent Scheib
Comment 1 2011-10-24 15:05:22 PDT
John Bates
Comment 2 2011-10-24 15:15:24 PDT
I discussed this with jbauman a bit, and we had some initial design ideas based on the constraint that the API must go through a vtable to get to chromium code. I'm sure more issues will come up so feel free to comment if you see any issues. Design goals: - fast (no vtable to check if a category is enabled). - support 0, 1 or 2 custom parameters - support begin, end, and instant event types. - limit code duplication (especially avoid struct layout duplication). namespace WebKit? { struct TraceCategoryInfo { const bool* isEnabled; const void* platformArgument; }; enum TraceEventType { TRACE_EVENT_BEGIN, TRACE_EVENT_END, TRACE_EVENT_INSTANT }; class TraceValue { ... yep, duplicate most/all of this class ... }; class PlatformTraceProvider { virtual TraceCategoryInfo getCategoryEnabledBool(const char* long_lived_string); virtual void addTraceEvent(TraceEventType type, const void* platformArgument, const char* name, const char* arg1name, TraceValue arg1val, ..., bool copy_strings); }; Then the macro would look something like: static TraceCategoryInfo category = PlatformTraceProvider::GetInstance()->getCategoryInfo(category_name); if (*category.isEnabled) PlatformTraceProvider::GetInstance()->addTraceEvent(..., category.platformArgument, ...); The implementation would have to convert between WebKit types (TraceEventType and TraceValue) and equivalent chromium types. The platformArgument would be set to point to the native chromium CategoryInfo struct, but WebKit code would not need to know what that type is.
Nat Duca
Comment 3 2011-10-24 15:21:23 PDT
I'm confused about the purpose of platformARgument. Why not just return a bool* from chromium, then use that where chromium macros typically use a TraceCategory*? How will you pass the TraceValue struct out without copying? That's the bit that stumped me.
John Bates
Comment 4 2011-10-24 15:46:20 PDT
(In reply to comment #3) > I'm confused about the purpose of platformARgument. Why not just return a bool* from chromium, then use that where chromium macros typically use a TraceCategory*? The TraceEvent class contains the TraceCategory pointer. Technically we could compute the TraceCategory pointer from the bool pointer, but I'm pretty sure I would be fired for even trying to get that past reviewers ;) > > How will you pass the TraceValue struct out without copying? That's the bit that stumped me. The platform glue code (chromium side impl of PlatformTraceProvider) would need to manually convert between the WebKit::TraceVaue and the base::debug::TraceValue. It would be great if we could just cast one to the other. The manual procedure is just to be safe in the event that the WebKit-side TraceValue becomes out of sync with the chromium version. Another question is whether we can put PlatformTraceProvider in a common WebKit directory so that we can trace everywhere (not just chromium platform code).
John Bates
Comment 5 2011-10-24 15:47:18 PDT
(In reply to comment #4) > (In reply to comment #3) > > I'm confused about the purpose of platformARgument. Why not just return a bool* from chromium, then use that where chromium macros typically use a TraceCategory*? > > The TraceEvent class contains the TraceCategory pointer. (eventually, it's used to get the category name and put that in the JSON) Technically we could compute the TraceCategory pointer from the bool pointer, but I'm pretty sure I would be fired for even trying to get that past reviewers ;) > > > > > How will you pass the TraceValue struct out without copying? That's the bit that stumped me. > > The platform glue code (chromium side impl of PlatformTraceProvider) would need to manually convert between the WebKit::TraceVaue and the base::debug::TraceValue. It would be great if we could just cast one to the other. The manual procedure is just to be safe in the event that the WebKit-side TraceValue becomes out of sync with the chromium version. > > Another question is whether we can put PlatformTraceProvider in a common WebKit directory so that we can trace everywhere (not just chromium platform code).
Nat Duca
Comment 6 2011-10-24 16:05:41 PDT
Ah, makes sense, thanks for sorting me out. Two high level things: 1. Lets hold off making this webcore wide. Amongst other things, we might need to go from category strings to something more like LogChannels so it makes more sense to other ports. 2. You'll need two types for every type you introduce in your solution: the WebCore type, and the type you add to WebKit/public/ that you use to get from WebKit back to Chromium. That reality might influence your API design a bit...
Note You need to log in before you can comment on or make changes to this bug.