Bug 110667 - [V8] meta: Insert TRACE_EVENT_STATE() macros into DOM attribute getters/setters/methods
Summary: [V8] meta: Insert TRACE_EVENT_STATE() macros into DOM attribute getters/sette...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on: 110669 110671 110676 110726 110765 110769 110781 110791 110794 110799
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-22 17:26 PST by Kentaro Hara
Modified: 2014-12-16 00:47 PST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2013-02-22 17:26:04 PST
To enable sampling profiling, we want to insert TRACE_EVENT_STATE() macros into DOM attribute getters/setters/methods. Specifically, CodeGeneratorV8.pm should generate the following code:

// DOM attribute getter
Handle<Value> xxxAttrGetterCallback(...) {
  TRACE_EVENT_STATE("DOMAttributeGetter");
  Handle<Value> value = xxxAttrGetter(...);
  TRACE_EVENT_STATE("Other");
  return value;
}

// DOM attribute getter (custom)
Handle<Value> xxxAttrGetterCallback(...) {
  TRACE_EVENT_STATE("DOMAttributeGetter");
  Handle<Value> value = xxxAttrGetterCustom(...);
  TRACE_EVENT_STATE("Other");
  return value;
}

// DOM attribute setter
void xxxAttrSetterCallback(...) {
  TRACE_EVENT_STATE("DOMAttributeSetter");
  xxxAttrSetter(...);
  TRACE_EVENT_STATE("Other");
}

// DOM attribute setter (custom)
void xxxAttrSetterCallback(...) {
  TRACE_EVENT_STATE("DOMAttributeSetter");
  xxxAttrSetterCustom(...);
  TRACE_EVENT_STATE("Other");
}

// DOM method
Handle<Value> xxxMethodCallback(...) {
  TRACE_EVENT_STATE("DOMMethod");
  Handle<Value> value = xxxMethod();
  TRACE_EVENT_STATE("Other");
  return value;
}

// DOM attribute method (custom)
Handle<Value> xxxMethodCallback(...) {
  TRACE_EVENT_STATE("DOMMethod");
  Handle<Value> value = xxxMethodCustom();
  TRACE_EVENT_STATE("Other");
  return value;
}
Comment 1 Kentaro Hara 2013-02-22 17:48:07 PST
And DOM constructors will look like:

// DOM constructor
Handle<Value> constructorCallback(...) {
  TRACE_EVENT_STATE("DOMConstructor");
  Handle<Value> value = constructor(...);
  TRACE_EVENT_STATE("Other");
  return value;
}

// DOM constructor (custom)
Handle<Value> constructorCallback(...) {
  TRACE_EVENT_STATE("DOMConstructor");
  Handle<Value> value = constructorCustom(...);
  TRACE_EVENT_STATE("Other");
  return value;
}
Comment 2 Ulfar Erlingsson 2013-03-04 16:05:08 PST
Kentaro, FYI, see the draft patch I have at https://bugs.webkit.org/show_bug.cgi?id=107207, which allows for a closure to be wrapped around any (or all) DOM getters/setters/methods.   The closures are created based on policy, and no overhead is incurred for DOM invocations without a closure.  It's work I've been doing with Ankur Taly on DOM activity logging for extensions, and bug https://bugs.webkit.org/show_bug.cgi?id=110779 where both you and Adam Barth commented on the relationship with your work on tracing.
Comment 3 Kentaro Hara 2013-03-04 16:13:11 PST
(In reply to comment #2)
> Kentaro, FYI, see the draft patch I have at https://bugs.webkit.org/show_bug.cgi?id=107207, which allows for a closure to be wrapped around any (or all) DOM getters/setters/methods.   The closures are created based on policy, and no overhead is incurred for DOM invocations without a closure.  It's work I've been doing with Ankur Taly on DOM activity logging for extensions, and bug https://bugs.webkit.org/show_bug.cgi?id=110779 where both you and Adam Barth commented on the relationship with your work on tracing.

Thanks for the info! I added comments on your bug.

In my understanding, the objectives of sampling TRACE_EVENTs and DOM closures are different, and thus both are needed, right? (See discussion in bug 110779)
Comment 4 Ulfar Erlingsson 2013-03-05 11:17:22 PST
(In reply to comment #3)
> (In reply to comment #2)
> > Kentaro, FYI, see the draft patch I have at https://bugs.webkit.org/show_bug.cgi?id=107207, which allows for a closure to be wrapped around any (or all) DOM getters/setters/methods.   The closures are created based on policy, and no overhead is incurred for DOM invocations without a closure.  It's work I've been doing with Ankur Taly on DOM activity logging for extensions, and bug https://bugs.webkit.org/show_bug.cgi?id=110779 where both you and Adam Barth commented on the relationship with your work on tracing.
> 
> Thanks for the info! I added comments on your bug.
> 
> In my understanding, the objectives of sampling TRACE_EVENTs and DOM closures are different, and thus both are needed, right? (See discussion in bug 110779)

Thanks for the comments.  I've replied on the other thread.

Yes, the two features are different, and both are needed.

However, in case you were interested, I just wanted to point out that closure mechanisms like those in bug 107207 could easily be used to dynamically add counters like the ones in this patch.  This could be advantageous, if you wanted to remove the overhead of tracing macros from the normal case use of DOM element wrappers), and it might make it easier for you to make later changes.
Comment 5 Brian Burg 2014-12-16 00:47:49 PST
Closing some V8-related work items.