Bug 110667 – [V8] meta: Insert TRACE_EVENT_STATE() macros into DOM attribute getters/setters/methods

• Component: WebCore JavaScript
• Status: RESOLVED INVALID

[Kentaro Hara]

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;
}

[Kentaro Hara]

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;
}

[Ulfar Erlingsson]

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.

[Kentaro Hara]

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

[Ulfar Erlingsson]

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

[Brian Burg]

Closing some V8-related work items.