\o/

Thank you for creating this, Elliott!

peria@ is looking into the binding performance issue.

I wonder whether some of the difference comes from dropping out of optimized code when we hit bindings? we can only optimize bindings that are always hit with the same type of wrapper currently...

might be worthwhile to look at the difference between real and fake dom with --js-flags=--nocrankshaft

Looking Performance Dashboard, revision	740688bec5c66af00debe117b6d375ecd212e570 had a regression in createElement.
https://chromeperf.appspot.com/report?sid=ae689d5a4c843101812273d654beb479d52a97962f5564574ce6c4fa2664a194&start_rev=380386&end_rev=386556
and it was fixed just after this issue was reported with following CL,
https://chromium.googlesource.com/chromium/src/+/607c6e18aae50211a534f964ebfdac47cdbf21be

After the change, creating DOM on Chrome is 2x slower than on Safari.

Safari 9:
innerHTML time: 23ms, nodes: 19601
Real bindings time: 48ms, nodes: 19601
Fake bindings time: 21ms, nodes: 19601

Chrome 51 Canary:
innerHTML time: 27ms, nodes: 19601
Real bindings time: 71ms, nodes: 19601
Fake bindings time: 30ms, nodes: 19601

The binding overhead is
Safari: 27ms
Chrome: 41ms

Chrome 51 with --js-flags="--nocrankshaft":
innerHTML time: 25ms, nodes: 19601
Real bindings time: 155ms, nodes: 19601
Fake bindings time: 109ms, nodes: 19601

The overhead is 46ms, similar to Chrome's score.

thanks for checking

that means that the overhead is indeed in the bindings.

Agreed. This overhead is independent from the optimization of JS code.

The markup file contained in #0 is complex, so I test it again with using a simple HTML.

var markup = '';
for (var i = 0; i < 100000; i++)
    markup += '<div></div>';

Then I get a different result from the original one.

Safari 9:
innerHTML time: 63ms
Real bindings time: 115ms
Fake bindings time: 150ms

Chrome 51 Canary:
innerHTML time: 51ms
Real bindings time: 191ms
Fake bindings time: 101ms

On Safari, fake bindings takes longer time than real bindings, so
it might be good to check if the difference between "real bindings time" and
"fake bindings time" shows an overhead of bindings.

Running an experiment that runs a tight loop of createElement(),
I got following conclusions

- createElement() on Chrome runs >2x slower than on Safari.  On Chrome, V8 GC takes 20-25% of runtime.
- The benchmark test in #0 does not show the overhead of bindings.  "Fake bindings" test does so many non-optimized operations that it takes longer time than "Real bindings" on Chrome and Safari.

I'm not sure we can conclude anything from running createElement in a tight loop. As soon as your microbenchmark is dominated by something that's not also a huge factor in the real benchmark, the conclusions are likely not applicable anymore.

I'm not sure I understand the point about the difference between fake and real bindings?

Re #12:
> As soon as your microbenchmark is dominated by something that's not also a huge factor
> in the real benchmark, the conclusions are likely not applicable anymore.
I agree on this point, but the real benchmark includes so many behaviors that we cannot
conclude other than "Chrome takes longer time than Safari to run this benchmark."

We have to make some micro benchmarks or simplify the real benchmarks to factor out
what can be the main cause of the slowness, and a tight loop of createElement() is a
sample test to check how long createElement() takes.



For the point about the defference between fake and real bindings,
#0 says that Chrome is 3x slower than Safari with comparing 37ms(=62-25) and 91ms(=114-23).
This result is based on the assumption that the real bindings does what the fake
bindings does and other works.
   (real bindings) = (fake bindings) + (other work)
If so, we cannot get shorter time with the real bindings than the fake bindings for
any tests using binding layers.  However, we can as I wrote in #10.

So we have to suspect that the assumption and the result can be wrong.

The benchmark was very specially crafted to model a real world scenario of calling setAttribute, createElement and appendChild with a range of attribute names, values and element names, and at varying depths (basically what real pages do). Creating 100k divs doesn't match what the page was actually doing, you never call appendChild or setAttribute, you also create the exact same object type over and over again.

It seems you're arguing that this isn't a bindings bug. Whether that's true or not Safari is still running the benchmark (even your simplified version) 50% or more faster.

I'd propose stepping back.

"Real bindings" and "fake bindings" are doing something complicated and thus hard to analyze. On the other hand, we already have many clearly slow benchmarks in Dromaeo. It would be easier to look into the benchmarks first.

According to peria-san's investigation (https://docs.google.com/spreadsheets/d/1oYwdUPCiCA0z8WMXbJLplg9vBNkHBKo3SjYB52ZkKzA/edit#gid=584539022), Chrome is >2x slower than Safari/Firefox in the following benchmarks.

DOM Attributes:	
	getAttribute
	element.property
	setAttribute
	element.property = value
	element.expando = value
	element.expando

DOM Modification:	
	createElement
	createTextNode
	appendChild
	insertBefore

DOM Query:	
	getElementById
	getElementById (not in document)
	getElementsByTagName(div)
	getElementsByTagName(p)
	getElementsByTagName(a)
	getElementsByTagName(*)
	getElementsByTagName (not in document)
	getElementsByName
	getElementsByName (not in document)

Notes:

- In some benchmarks Firefox is extremely faster than Chrome. This is because Firefox implements an optimization to treat side-effect-free DOM attributes/methods as dead code. For example, Firefox treats element.id as dead code.

- I guess that most of the slowness is caused by C++ DOM operations rather than V8 bindings. For example, Chrome is only 1.5x slower than Safari in element.firstChild. This means that 1.5x is the maximum slowness caused by V8 bindings. If Chrome is 3x slower than Safari in some benchmark, it means that the slowness is caused by something else than V8 bindings. (You might want to write a benchmark that repeatedly accesses element.firstChild that returns undefined. If Chrome is X% slower than Safari, it would mean that X is the maximum slowness caused by V8 bindings.)

The slowness is not the C++ DOM, I've extensively profiled that and we're on par with Safari. See also the innerHTML case which is executing the same C++ operations but with the added parser overhead. Note that Safari and Chrome are about the same.

.firstChild is not a good example. It doesn't need  argument validation, string conversions, is trivially inlinable in some engines, etc. Real pages don't use .firstChild very much either. I don't think the tight loops in dromaeo really model what's going on in real pages. This benchnark is intentionally crafted to model real pages.

The problem really is the bindings or v8, our C++ is definitely not 2-3x slower I've spent years profiling that.

I think we need to choose benchmarks we should optimize.

It is problematic that Chrome is consistently 2 - 5x slower than Safari in Dromaeo (even though some of the benchmarks are not realistic ones, it would be problematic that Chrome is 2- 5x slower in getElementByXXX). It is also problematic that your parsing benchmark is slow.

The question is what benchmark we should look at first.

IMHO I'd suggest looking at Dromaeo because 1) it is a widely-recognized benchmark that matters for marketing, 2) it is simple and easy to analyze, and 3) it tests common paths of V8 and V8 bindings (if the performance problem is in V8 bindings, we'll be able to fix the problem by fixing performance problems in Dromaeo).

In choosing the path, please consider that the benchmark on this bug is written by an engineer who:

1) works really close with you and is eager to help with walkthroughts, deep dives, and generally making progress toward better state;

2) deeply understands front-end performance and has a ton of experience writing front-end code;

3) deeply understands how Blink works and where the potential performance problems are.

To put it differently, would we go faster if we didn't treat benchmarks as a black box, and instead looked at it as a capture of esprehn@'s brain and then dissecting it (the capture, not the brain :-O ) -- together with esprehn@?

For example, when we feel like the code is doing something complicated, in a white box benchmark, we can just tweak it and make it less complicated. In a black box benchmarks, you are stuck with assumptions made by the author.

Finally, let's not worry about marketing just yet. That's something we can tackle later. First, let's focus on identifying the key problems that need addressing. My intuition is that these are the same problems that will help making all benchmarks of the same class go faster.

I'd rather not optimize dromaeo - we know that we can speed up dromaeo a lot without having measurable impact on websites. It's also true that there are things that improve both dromaeo and websites, but in general, I think it's not the best optimization goal.

Chatted with Elliott. Yeah, it sounds more reasonable to focus on the parsing benchmark here :D

peria-san: Would it be helpful to get numbers for the following scenarios for example?

a) Use a markup file that contains a shadow tree of <div>s.

markup = '<div></div><div></div><div></div>...';

b) Use a markup file that contains a shadow tree of <div>s that have attributes.

markup = '<div id=aaa></div><div id=aaa></div><div id=aaa></div>...';

c) Use a markup file that contains a deep tree of <div>s.

markup = '<div><div><div>...</div></div></div>';

d) Use a markup file that contains a deep tree of <div>s that have attributes.

markup = '<div id=aaa><div id=aaa><div id=aaa>...</div></div></div>';

e) In Chrome, add more redundant instructions to appendChildMethodForMainCallback, createElementMethodForMainWorld and setAttributeMethod and see how it affects performance of real dom bindings.

f) In Chrome, reduce instructions from appendChildMethodForMainCallback, createElementMethodForMainWorld and setAttributeMethod and see how it affects performance of real dom bindings. For example, you can remove ExceptionState, argument length checks, CustomElementProcessingStack etc.

I hope that performance results for a) - f) will give us some hints. Overall I want to clarify whether the "binding overhead" comes from the overhead when V8 calls into V8 bindings or the overhead when V8 bindings calls into core/. If the culprit is the latter, I want to clarify what the culprit is (argument checks, CustomElementProcessingStack, toV8 etc).

Let me share the current progress for this issue:
- In the benchmark in #0, we have some candidates to see; createElement(), setAttribute(), appendChild(), and parsing HTML.
- With benchmarks in #20, appendChild(), createElement(), and setAttribute() are all 2.5x-3x slower on Chrome than on Safari.
- Digging into createElement(),
  - creating DOM objects has no large performance difference between Blink and WebKit.
  - v8::Persistent<>::Reset() is the largest time-consuming part.

The details and few other investigation is written in this working note;
https://docs.google.com/document/d/1DZBWwDkOrIh76NydfQZEmRT8BXEjfcvhVIWuA7F3PFQ/edit?usp=sharing

Thanks for writing up the document! The analysis looks reasonable, and I agree with you that v8::Persistent::Reset is key to optimize the benchmark.

As discussed offline, let's analyze a bit more inside v8::Persistent::Reset so that we can provide more actionable feedback to the V8 team.

To also chime in on the conversation:
 
We come from a software company that is currently developing a mobile application that utilizes chrome webview. Due to our hardware restrictions, we have been experiencing major performance issues. As a result, we are looking for every possible optimization we can make to speed up our product. As was mentioned previously, we ran a variety of web browsers through Dromaeo to analyze the performance. Reviewing the results and doing some research led us to this exact issue. I Understand that you are not interested in chasing benchmarks, but rather improving real world scenarios. So to give some background, our product leverages the Sencha Ext framework to create complex and highly responsive user interfaces. This means that we heavily depend on DOM Modification methods such as createElement and appendChild. We notice that just creating the login page for our web application takes roughly twice as long on Chrome than Safari when running on the same exact hardware.  In an attempt to try to replicate our use case, I created a variety of simple test cases in JSFiddle and ran them through both Chrome and Safari to showcase the differences. Basically, we noticed DOM Modification operations are usually 2 to 3 times slower in Chrome than Safari. The results are located within the following google doc: https://docs.google.com/spreadsheets/d/1sBegQcifI6m7TN1WmWJWlr78c-gWnpa59hb8WJfiW5A/edit?pref=2&pli=1#gid=0

 
We are very happy to see that this issue is being actively investigated and are very interested in any resolutions. Thanks!

Thanks for reporting the issue, slawski!

Yes, now we're focusing on making createElement & appendChild faster. It will speed up both Elliott's parsing benchmark and Dromaeo.

peria's latest result is written in this doc (https://docs.google.com/document/d/1DZBWwDkOrIh76NydfQZEmRT8BXEjfcvhVIWuA7F3PFQ/edit#heading=h.vd5s5bssnslx).

Summary:

- createElement is the largest bottleneck of Elliott's parsing benchmark.
- The breakdown of createElement is as follows:

v8::Persistent::Reset: 25%
minor GC: 21%
v8::Object::Clone: 10%

v8::Persistent::Reset is called by this code (https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/core/v8/ScriptWrappable.h&q=scriptwrappable.h&sq=package:chromium&type=cs&l=106). Is there anything we can make it faster? The persistent handle is empty before the code calls Reset, so the Reset isn't calling V8::DisposeGlobal.

It looks like that https://codereview.chromium.org/1944363002/ speeded up the minor GC a lot.

https://codereview.chromium.org/1944363002/ improved blink_perf.bindings/create-element it by 13%-20%: https://chromeperf.appspot.com/group_report?rev=392613
This probably means that "minor GC" item is done.

I am looking into m_wrapper.Reset(isolate, wrapper). It probably spends time in creating a new global handle. Maybe we can speed it up.

Any luck here? I see 55ms for WebKit nightly and 85ms for Chrome Canary. :)

I wrote up my findings about Reset() performance in  crbug.com/619869 

If we re-implement global handle management in V8, we can probably speed up Reset by 2x-3x, which will translate to 12%-16% improvement in benchmark from #25.

That is a large project though.

Re #28;
I think the numbers are reasonable.
After my measurement in #6, ulan@'s change in #25 is a significant change.

Re #30;
It sounds very good to me.  If you can achieve 12% time reduction in Reset(), it is larger than removing Clone().

peria@, esprehn@, do you think it's worthwhile to add the parsing benchmark to telemetry?

Sounds like a good idea.

When esprehn was here he said he'd look into it.

Picking up the four methods mentioned in #0, I measured their time distribution in the benchmark
using Instruments.

> setAttribute   38%
> createElement  30%
> createTextNode 27%
> appendChild     5%
(note: I ran the test 10 times, and I ignored time spent in other call trees.)

And some more details in each method are following.

* in setAttribute
82% blink::Element::setAttribute()
-> 77% blink::Element::appendAttributeInternal()
--> 40% blink::Element::attributeChanged()
---> 16% blink::Element::sytleAttributeChange()
15% blink::v8StringToWebCoreString

* in createElement
62% blink::Document:createElement()
14% blink::Node::wrap()
-> 5% v8::Object::Clone
 8% blink::V8DOMWrapper::associateObjectWithWrapper()
-> 4% v8::V8::GlobalizeReference()  (this method is called in v8::Persistent::Reset())
11% blink::v8StringToWebCoreString

* in createTextNode
67% blik::Text::create()
14% blink::Node::wrap()
-> 4% v8::Object::Clone
10% blink::V8DOMWrapper::associateObjectWithWrapper()
-> 5% v8::V8::GlobalizeReference()  (this method is called in v8::Persistent::Reset())
 4% blink::v8StringToWebCoreString

* in appendChild
68% blink::ContainerNode::appendChild()
18% blink::V8PerIsolateData::hasInstance()


If this analysis shows correct time distribution, speeding up inside V8 do not improve
performance so much.
And comparing with WebKit, Blink seems to be 2x-3x slower even in DOM operations.
It does not meet the assumption that we had in #0, though.
(FYI, time to run the benchmark for 10 times is 613ms on Blink and 385ms on WebKit.)

How did you get the time distribution?

I agree with the conclusion that we should optimize Blink-side DOM operations and v8StringToWebCoreString, but I'm not sure about the conclusion that improving V8 won't contribute to the performance. For example, cbruni and jochen are now trying to remove Object::Clone but they're hitting some performance regression on createElement(), which implies that the overhead of Object::Clone matters.

We're not 2-3x slower in DOM operations because innerHTML is the same speed. :) I'm pretty sure this is an issue with bindings or v8. What benchmark makes you think our C++ DOM is slower?

In your profile:
- Instruments doesn't understand the generated code from v8, that's all those missing symbols.
- V8PerIsolateData::hasInstance is lots of time, those are all hash table lookups. We need to remove those hash tables and just lookup the type directly from the object type.
- v8StringToWebCoreString is lots of time, JSC doesn't need to copy non-rope strings, the parser is doing createElement(buffer.substring(a, b)) which probably means we're doing constant string copies that Safari gets to avoid.
- Note the large amount of time spent in Node::wrap, V8DOMWrapper::associateObjectWithWrapper, V8::GlobalizeReference.

I agree with esprehn@.

associateObjectWithWrapper for instance does two consecutive calls to set the internal fields of the wrapper, this could probably be replaced with one call setting several properties at once.

I'm working on improving the wrapper creation taking all sorts of shortcuts.
With the blink caches in place we manage to hit ~500 runs/sec, if I disable them we're at ~110 runs/sec. If I avoid unnecessary API round trips but still rely on V8 to do most of the work I end up at 300 runs/sec, so there is some room for improvement.

Re #39
I used Intruments in XCode, which is a common performance analyzer on MacOSX.
I ran Elliott's benchmark in #0 for 10 times with it.

Re #40
Hmm, I think what "DOM operations" was/is different between you and me.
For example for "setAttribute()" in JS, I thought "DOM operations" included Element::setAttribute()
but it was not called in the innerHTML test.  Some callee methods (e.g. Element::attributeChanged())
were called in both tests.  You meant these callee methods with "DOM operations", right?

Is innerHTML really not calling Element::setAttribute() (or Element::setAttributeInternal())? How is it setting attributes on elements?

I can run innerHTML test with adding a CHECK(false) in Element::setAttribute().
It means the test does not go through the method.

In innerHTML, attributes are set through Element::parserSetAttributes() called by HTMLDocumentParser and HTMLTreeBuilder.

Yes, the parser takes a slightly different path when setting attributes, it does Element::parserSetAttributes instead of Element::setAttribute. There probably is stuff to be optimized inside Element::setAttribute itself, though your profile shows 40% is in attributeChanged which is in both paths, and 15% is in string copies which is a bindings issue. It's possible that other 45% is a difference with Safari:

WebKit:
https://github.com/WebKit/webkit/blob/master/Source/WebCore/dom/Element.cpp#L1163

Blink:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Element.cpp?q=Element::setAttr&sq=package:chromium&l=1092

They've simplified this code some and use ascii comparisons. Your profile shows 38% in setAttribute. Ignoring the 40% shared cost and 15% string copies that's 17% inside setAttribute directly. If we make that 2x faster we get 8.5% back on the benchmark. If you have ideas there we should certainly optimize it, but I'd say lets look at the bindings first since it generalizes to lots more stuff in the platform, ex. the new CSS Typed OM, WebGL, etc.

I'm also pretty skeptical that there's a enough stuff in the C++ DOM to explain our 2-3x difference since folks have been complaining about us being slower (and I've measured it slower) for years, long before the fork even when we shared the same C++ code. Certainly if we're within 20% of Safari someday it could be that. :)

+1 to Elliott's point.

I'm skeptical about the time distribution in #38 though. It doesn't explain the fact that innerHTML performs mostly equal between Safari and Chrome or the fact that cbruni's object creation optimization has a big impact on the performance of createElement.

v8StringToWebCoreString is consuming a lot of time in #38, but is it externalizing V8 strings?

> v8StringToWebCoreString is consuming a lot of time in #38, but is it externalizing V8 strings?

No.
It is creating AtomicString instances and registering them into AtomicStringTable.

Just in case, would you try to call v8StringToWebCoreString multiple times and confirm that it really affects performance? If it affects performance, we can confidently say that v8StringToWebCoreString is (one of) the bottleneck.

yutak's string hashing function optimization & WTF::ThreadSpecific optimization might be helpful to make it faster.

I ran Elliott's parsing benchmark on Linux and got a pretty different result.

I profiled the following code:

var markup = /* the contents of wikipedia-markup.txt */;
for (var iter = 0; iter < 100; iter++) {
  var parser = new DomParser(markup, RealDomParserBindings);
  parser.parse();  // Profile this function.
  gc(); gc(); gc();
}

The result is as follows:

Total time: 340 ms
createElement: 30 ms
createTextNode: 30 ms
appendChild: 140 ms
setAttribute: 60 ms
V8 minor GC: 2 ms
V8 major GC: 0 ms
Oilpan GC: 0 ms
Others: 80 ms (executing JS to parse the html etc)

I uploaded a CL for profiling the benchmark: https://codereview.chromium.org/2197923002

peria-san: Would you try the patch on Safari and Chrome on your Mac?

BTW, we should upstream Elliott's parsing benchmark to ToT.

> BTW, we should upstream Elliott's parsing benchmark to ToT.
Do you mean the  issue 625986 ?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31cc1d1f5b9f4bdcbc69cbc788ffc31696895a60

commit 31cc1d1f5b9f4bdcbc69cbc788ffc31696895a60
Author: cbruni <cbruni@chromium.org>
Date: Fri Aug 05 22:25:52 2016

[bindings] Use new SetAlignedPointerInInternalFields V8 API

This should help to reduce the overhead of setting up DOM wrappers.

BUG=chromium:630217,chromium:595492

Review-Url: https://codereview.chromium.org/2184113002
Cr-Commit-Position: refs/heads/master@{#410191}

[modify] https://crrev.com/31cc1d1f5b9f4bdcbc69cbc788ffc31696895a60/third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.h

 Issue 374903  has been merged into this issue.

I'll take ownership of this bug. It's similar to other bugs I own.

Update of this benchmark on my 2013 Mac Pro Trashcan:

Chrome Version 68.0.3440.106 (Official Build) (64-bit):
innerHTML time: 33ms, nodes: 19601
Real bindings time: 101ms, nodes: 19601
Fake bindings time: 43ms, nodes: 19601

Safari 11.1.2 (13605.3.8):
innerHTML time: 29ms, nodes: 19601
Real bindings time: 75ms, nodes: 19601
Fake bindings time: 15ms, nodes: 19601

Firefox 62.0b17 (64-bit):
innerHTML time: 28ms, nodes: 19601
Real bindings time: 115ms, nodes: 19601
Fake bindings time: 96ms, nodes: 19601

Chrome is not too far off Safari on this device, looks like 35% slower? I am surprised at how slow the "Fake bindings time" is though. That's a pure JS benchmark with no bindings that's testing how fast an html parser that creates mock DOM nodes can be. Looks like v8 is 2.86x slower than JSC on that one.

I have a 2017 Macbook Pro I can check on later as well.