Re: [webkit-dev] Making more use of ScriptWrappable
On Nov 5, 2012, at 5:07 PM, Adam Barth aba...@webkit.org wrote:
On Thu, Nov 1, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote:
Sounds like a good idea. Three additional thoughts:
(1) It would be best to choose the objects to apply this to in some
data-driven way.
Do you have a suggestion for what data to use? As far as I can tell,
adding ScriptWrappable as a base class is a win whenever at least half
of the instances of the object have JavaScript wrappers (in the main
world):
It's clearly an all-round win for speed and memory when this condition is true.
I imagined some straightforward instrumentation to find classes where this
condition is true (or nearly true) on some representative set of websites.
Applying ScriptWrappable to these classes is definitely pure win.
When the condition is not true (wrapper exists in main world significantly less
than half the time), it's a size-speed tradeoff. It may be worth it in cases if
the memory cost is low and the speed gain is high (for example, one object per
document, but its wrapper is accessed often). That may require some judgment
calls.
(3) I suspect that we can handle this without adding an IDL attribute at
all. C++ overloaded functions could let the bindings do something different
for objects that inherit ScriptWrappable from ones that do not in a generic
way, without having to explicitly tell the bindings layer about the ways to
do it. Consider the ways unwrap() and toJS() are done. We don't have to say
anything special in the IDL or have any interface-specific knowledge in the
bindings, C++ overloading takes care of it.
Thanks for the suggestion. I got this work (at least for the V8
bindings---JSC is next on my list). To make something
ScriptWrappable, you just need to add ScriptWrappable as a base class:
-class DOMImplementation {
+class DOMImplementation : public ScriptWrappable {
I'm not super excited about the name given that all DOM objects are
wrappable by script. If folks have thoughts about a better name, I'd
appreciate suggestions.
I agree with you that the name could use improvement. The only suggestion I can
think of that is not ridiculously verbose would be InlineWrappable, but I am
not sure if that is more clear overall.
Cheers,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
To update this thread: I've now got this working in the V8 bindings.
The next step is to make this work in the JSC bindings. If you're
interested in the details, the work will occur on
https://bugs.webkit.org/show_bug.cgi?id=101279.
On Thu, Nov 1, 2012 at 10:51 AM, Alexey Proskuryakov a...@apple.com wrote:
Do you have a rough estimate of how large of a win we are talking about?
As a simple example, adding ScriptWrappable as a base class for
DOMImplementation makes document.implementation 23% faster, at least
as measured with the V8 bindings (see
https://bug-101279-attachments.webkit.org/attachment.cgi?id=172440).
Although I doubt that document.implementation itself is a performance
bottleneck, using ScriptWrappable more widely seems likely to improve
both performance and memory usage.
On Thu, Nov 1, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote:
Sounds like a good idea. Three additional thoughts:
(1) It would be best to choose the objects to apply this to in some
data-driven way.
Do you have a suggestion for what data to use? As far as I can tell,
adding ScriptWrappable as a base class is a win whenever at least half
of the instances of the object have JavaScript wrappers (in the main
world):
A) It's always faster to get and set the JavaScript wrapper with
ScriptWrappable.
B) In terms of memory, we pay 1*sizeof(void*) for every instance with
ScriptWrappable compared to 2*sizeof(void*) for every instance that
has a JavaScript wrapper in the non-ScriptWrappable case (discounting
the fact that Hashtable actually seems to keep a constant fraction of
its buckets free).
(2) If we have an IDL attribute, I think it should be named by the effect it
has, not the possible conceptual-level reason for applying it.
[ScriptWrappable] or [InlineWrapper] or something. Because it's not a
judgment call, it is a statement about the code.
Turns out we don't need the IDL attribute (see the next question).
(3) I suspect that we can handle this without adding an IDL attribute at all.
C++ overloaded functions could let the bindings do something different for
objects that inherit ScriptWrappable from ones that do not in a generic way,
without having to explicitly tell the bindings layer about the ways to do it.
Consider the ways unwrap() and toJS() are done. We don't have to say anything
special in the IDL or have any interface-specific knowledge in the bindings,
C++ overloading takes care of it.
Thanks for the suggestion. I got this work (at least for the V8
bindings---JSC is next on my list). To make something
ScriptWrappable, you just need to add ScriptWrappable as a base class:
-class DOMImplementation {
+class DOMImplementation : public ScriptWrappable {
I'm not super excited about the name given that all DOM objects are
wrappable by script. If folks have thoughts about a better name, I'd
appreciate suggestions.
Thanks,
Adam
On Thu, Nov 1, 2012 at 10:36 AM, Adam Barth aba...@webkit.org wrote:
We currently use two different approaches for associating JavaScript
wrappers with DOM objects. For some objects, we store the wrapper
inline in the object itself by making object inherit from
ScriptWrappable. For other types of objects, we use a HashMap to
translate the object into a JavaScript wrapper.
Whether to use ScriptWrappable or a HashMap is a trade-off that
depends on the workload. For DOM objects that rarely have a
JavaScript wrapper, using a HashMap is more memory efficient because
we don't need to store a large number of null pointers in objects that
do not have wrappers. By contrast, if an object almost always has a
JavaScript wrapper, using ScriptWrappable is both faster (because we
avoid the hash table lookup) and uses less memory (because we don't
need to store both the key and the value in the HashMap---we just need
to store the value in the object itself).
Today, we use ScriptWrappable for Nodes only, but we would benefit by
making more use of ScriptWrappable, particularly for DOM objects that
almost always have JavaScript wrappers. For example, XMLHttpRequest
objects exist only when created by script, which means that every
XMLHttpRequest object has a JavaScript wrapper.
My plan is to introduce an interface-level IDL attribute named
something like [OftenHasJSWrapper] that informs the code generator
that the object inherits from ScriptWrappable and that we should make
use of the inline wrapper. We can then deploy this attribute as
appropriate throughout WebCore to reduce memory usage and improve
performance.
Please let me know if you have any feedback.
Thanks!
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Nov 2, 2012, at 12:45 AM, Adam Barth aba...@webkit.org wrote: On Thu, Nov 1, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote: (3) I suspect that we can handle this without adding an IDL attribute at all. C++ overloaded functions could let the bindings do something different for objects that inherit ScriptWrappable from ones that do not in a generic way, without having to explicitly tell the bindings layer about the ways to do it. Consider the ways unwrap() and toJS() are done. We don't have to say anything special in the IDL or have any interface-specific knowledge in the bindings, C++ overloading takes care of it. That's a good idea. I'll see if we can avoid the IDL attribute. I wonder if we can do the same for the ActiveDOMObject IDL attribute, which similarly announces the presence of a base class. Good thought. ActiveDOMObject is a little different, because it implies some Web-observable behavior rather than just something about the mechanics of binding. But despite that, it's probably still better not to mention it in IDL if there is no technical need to do so. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 10:51 AM, Alexey Proskuryakov a...@apple.com wrote: 01.11.2012, в 10:36, Adam Barth aba...@webkit.org написал(а): Today, we use ScriptWrappable for Nodes only, but we would benefit by making more use of ScriptWrappable, particularly for DOM objects that almost always have JavaScript wrappers. For example, XMLHttpRequest objects exist only when created by script, which means that every XMLHttpRequest object has a JavaScript wrapper. This looks like a great idea for investigation to me. Do you have a rough estimate of how large of a win we are talking about? I don't have numbers currently, but I'm happy to gather some. My recollection is that the win for nodes was substantial on DOM benchmarks. I would expect the win to be fairly large for DOM objects like NodeList where script is likely to access properties like length and item frequently (e.g., in a loop). For example, XMLHttpRequests are probably not numerous enough to matter. Also, this would not be an improvement for XMLHttpRequest objects created in secondary worlds. Yeah. This is a main-world optimization only. Even for Nodes, we use a HashMap for secondary worlds. Adam I expect there to be cases where the win is more clear though. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
Sounds like a good idea. For some objects, we store the wrapper inline in the object itself by making object inherit from ScriptWrappable. For other types of objects, we use a HashMap to translate the object into a JavaScript wrapper. The current situation is not either ScriptWrappable or the HashMap. Currently we store all wrappers into the HashMap and store some wrappers into ScriptWrappable as a cache. In other words, wrappers stored in ScrpitWrappable are also stored in the HashMap. The fact that the HashMap knows all wrappers is being used at some places e.g. when GC enumerate wrappers. Thanks to the recent efforts of Adam Barth, V8 bindings are going to remove the need to enumerate all wrappers, and thus the HashMap does not need to store all wrappers. On the other hand, what about JSC bindings? Either way, I would support the idea of introducing ScriptWrappable for more DOM objects. On Thu, Nov 1, 2012 at 6:36 PM, Adam Barth aba...@webkit.org wrote: We currently use two different approaches for associating JavaScript wrappers with DOM objects. For some objects, we store the wrapper inline in the object itself by making object inherit from ScriptWrappable. For other types of objects, we use a HashMap to translate the object into a JavaScript wrapper. Whether to use ScriptWrappable or a HashMap is a trade-off that depends on the workload. For DOM objects that rarely have a JavaScript wrapper, using a HashMap is more memory efficient because we don't need to store a large number of null pointers in objects that do not have wrappers. By contrast, if an object almost always has a JavaScript wrapper, using ScriptWrappable is both faster (because we avoid the hash table lookup) and uses less memory (because we don't need to store both the key and the value in the HashMap---we just need to store the value in the object itself). Today, we use ScriptWrappable for Nodes only, but we would benefit by making more use of ScriptWrappable, particularly for DOM objects that almost always have JavaScript wrappers. For example, XMLHttpRequest objects exist only when created by script, which means that every XMLHttpRequest object has a JavaScript wrapper. My plan is to introduce an interface-level IDL attribute named something like [OftenHasJSWrapper] that informs the code generator that the object inherits from ScriptWrappable and that we should make use of the inline wrapper. We can then deploy this attribute as appropriate throughout WebCore to reduce memory usage and improve performance. Please let me know if you have any feedback. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev -- Kentaro Hara, Tokyo, Japan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 2:09 PM, Kentaro Hara hara...@google.com wrote: ... Thanks to the recent efforts of Adam Barth, V8 bindings are going to remove the need to enumerate all wrappers, and thus the HashMap does not need to store all wrappers. On the other hand, what about JSC bindings? JSC has a map per world, and the main world wrappers are stored in the ScriptWrappable, not in the map. Only Node's are ScriptWrappable it seems. http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/JSDOMBinding.hl=137 http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/JSNodeCustom.hl=42 I'm not sure why setInlineCachedWrapper takes a Node* specifically, it should just take a ScriptWrappable* so Adam's change works for JSC too. - E ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 11:21 AM, Elliott Sprehn espr...@chromium.org wrote: On Thu, Nov 1, 2012 at 2:09 PM, Kentaro Hara hara...@google.com wrote: ... Thanks to the recent efforts of Adam Barth, V8 bindings are going to remove the need to enumerate all wrappers, and thus the HashMap does not need to store all wrappers. Yes, the last patch in that series landed a few minutes before I sent this email. On the other hand, what about JSC bindings? JSC has a map per world, and the main world wrappers are stored in the ScriptWrappable, not in the map. Only Node's are ScriptWrappable it seems. http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/JSDOMBinding.hl=137 http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/js/JSNodeCustom.hl=42 I'm not sure why setInlineCachedWrapper takes a Node* specifically, it should just take a ScriptWrappable* so Adam's change works for JSC too. Yes, this should be straightforward for the JSC bindings. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 11:04 AM, Adam Barth aba...@webkit.org wrote: On Thu, Nov 1, 2012 at 10:51 AM, Alexey Proskuryakov a...@apple.com wrote: 01.11.2012, в 10:36, Adam Barth aba...@webkit.org написал(а): Today, we use ScriptWrappable for Nodes only, but we would benefit by making more use of ScriptWrappable, particularly for DOM objects that almost always have JavaScript wrappers. For example, XMLHttpRequest objects exist only when created by script, which means that every XMLHttpRequest object has a JavaScript wrapper. This looks like a great idea for investigation to me. Do you have a rough estimate of how large of a win we are talking about? I don't have numbers currently, but I'm happy to gather some. My recollection is that the win for nodes was substantial on DOM benchmarks. I would expect the win to be fairly large for DOM objects like NodeList where script is likely to access properties like length and item frequently (e.g., in a loop). Definitely. The last I checked, we spent 3-5% just looking for the wrapper in the hash map when iterating over NodeList items. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On the other hand, what about JSC bindings? JSC bindings do not store the wrapper in two places. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 12:41 PM, Geoffrey Garen gga...@apple.com wrote: On the other hand, what about JSC bindings? JSC bindings do not store the wrapper in two places. I'm not sure I fully understand your message. JSC uses ScriptWrappable for Nodes and uses a HashMap for non-Nodes (in the main world), just like V8 does. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Nov 1, 2012, at 6:36 PM, Adam Barth aba...@webkit.org wrote: We currently use two different approaches for associating JavaScript wrappers with DOM objects. For some objects, we store the wrapper inline in the object itself by making object inherit from ScriptWrappable. For other types of objects, we use a HashMap to translate the object into a JavaScript wrapper. Whether to use ScriptWrappable or a HashMap is a trade-off that depends on the workload. For DOM objects that rarely have a JavaScript wrapper, using a HashMap is more memory efficient because we don't need to store a large number of null pointers in objects that do not have wrappers. By contrast, if an object almost always has a JavaScript wrapper, using ScriptWrappable is both faster (because we avoid the hash table lookup) and uses less memory (because we don't need to store both the key and the value in the HashMap---we just need to store the value in the object itself). Today, we use ScriptWrappable for Nodes only, but we would benefit by making more use of ScriptWrappable, particularly for DOM objects that almost always have JavaScript wrappers. For example, XMLHttpRequest objects exist only when created by script, which means that every XMLHttpRequest object has a JavaScript wrapper. My plan is to introduce an interface-level IDL attribute named something like [OftenHasJSWrapper] that informs the code generator that the object inherits from ScriptWrappable and that we should make use of the inline wrapper. We can then deploy this attribute as appropriate throughout WebCore to reduce memory usage and improve performance. Please let me know if you have any feedback. Sounds like a good idea. Three additional thoughts: (1) It would be best to choose the objects to apply this to in some data-driven way. (2) If we have an IDL attribute, I think it should be named by the effect it has, not the possible conceptual-level reason for applying it. [ScriptWrappable] or [InlineWrapper] or something. Because it's not a judgment call, it is a statement about the code. (3) I suspect that we can handle this without adding an IDL attribute at all. C++ overloaded functions could let the bindings do something different for objects that inherit ScriptWrappable from ones that do not in a generic way, without having to explicitly tell the bindings layer about the ways to do it. Consider the ways unwrap() and toJS() are done. We don't have to say anything special in the IDL or have any interface-specific knowledge in the bindings, C++ overloading takes care of it. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
On Thu, Nov 1, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote: On Nov 1, 2012, at 6:36 PM, Adam Barth aba...@webkit.org wrote: We currently use two different approaches for associating JavaScript wrappers with DOM objects. For some objects, we store the wrapper inline in the object itself by making object inherit from ScriptWrappable. For other types of objects, we use a HashMap to translate the object into a JavaScript wrapper. Whether to use ScriptWrappable or a HashMap is a trade-off that depends on the workload. For DOM objects that rarely have a JavaScript wrapper, using a HashMap is more memory efficient because we don't need to store a large number of null pointers in objects that do not have wrappers. By contrast, if an object almost always has a JavaScript wrapper, using ScriptWrappable is both faster (because we avoid the hash table lookup) and uses less memory (because we don't need to store both the key and the value in the HashMap---we just need to store the value in the object itself). Today, we use ScriptWrappable for Nodes only, but we would benefit by making more use of ScriptWrappable, particularly for DOM objects that almost always have JavaScript wrappers. For example, XMLHttpRequest objects exist only when created by script, which means that every XMLHttpRequest object has a JavaScript wrapper. My plan is to introduce an interface-level IDL attribute named something like [OftenHasJSWrapper] that informs the code generator that the object inherits from ScriptWrappable and that we should make use of the inline wrapper. We can then deploy this attribute as appropriate throughout WebCore to reduce memory usage and improve performance. Please let me know if you have any feedback. Sounds like a good idea. Three additional thoughts: (1) It would be best to choose the objects to apply this to in some data-driven way. (2) If we have an IDL attribute, I think it should be named by the effect it has, not the possible conceptual-level reason for applying it. [ScriptWrappable] or [InlineWrapper] or something. Because it's not a judgment call, it is a statement about the code. (3) I suspect that we can handle this without adding an IDL attribute at all. C++ overloaded functions could let the bindings do something different for objects that inherit ScriptWrappable from ones that do not in a generic way, without having to explicitly tell the bindings layer about the ways to do it. Consider the ways unwrap() and toJS() are done. We don't have to say anything special in the IDL or have any interface-specific knowledge in the bindings, C++ overloading takes care of it. That's a good idea. I'll see if we can avoid the IDL attribute. I wonder if we can do the same for the ActiveDOMObject IDL attribute, which similarly announces the presence of a base class. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
