Attachment #293901 for bug #164356

View Details Raw Unified Return to Bug 164356

| Differences between



2016-11-04  Yusuke Suzuki  <utatane.tea@gmail.com>

        [DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById
        https://bugs.webkit.org/show_bug.cgi?id=164356

        Reviewed by Filip Pizlo.

        This patch implements DOMJIT::Signature annotation for getElementById.
        Since getElementById is also implemented in DocumentFragment, we implement
        the branchIfDocumentFragment/branchIfNotDocumentFragment for that.

        In dromaeo, we have a test like this.

        test( "getElementById", function(){
            for ( var i = 0; i < num * 30; i++ ) {
                ret = document.getElementById("testA" + num).nodeType;
                ret = document.getElementById("testB" + num).nodeType;
                ret = document.getElementById("testC" + num).nodeType;
                ret = document.getElementById("testD" + num).nodeType;
                ret = document.getElementById("testE" + num).nodeType;
                ret = document.getElementById("testF" + num).nodeType;
            }
        });

        In the above test, JSC already knows the following things.

        1. Since nodeType is now handled as CallDOMGetter, we know that it is pure.
        2. getElementById look up becomes PureGetById since document is impure object. But it is kept as PureGetById. So it does not write DOMState.
        3. `"testA" + num` will be converted to constant string.
        4. CallDOM for getElementById said it just reads(DOMState:DOM). And it saids that it returns the same value as long as DOMState is not clobbered.
        5. CheckCell leading CallDOM ensures the inlined getElementById node. (CallDOM node).

        The key thing is that no node clobbers DOMState during the loop. So CallDOM & CallDOMGetter can be hoisted.
        This improves dom-query significantly. Dromaeo dom-query getElementById becomes 40x faster (247796 v.s. 6197).
        Dromaeo dom-query getElementById (not in document) becomes 89x faster (630317.8 v.s. 7066.).

        Tests: js/dom/domjit-function-get-element-by-id-changed.html
               js/dom/domjit-function-get-element-by-id-licm.html
               js/dom/domjit-function-get-element-by-id.html

        * dom/NonElementParentNode.idl:
        * domjit/DOMJITCheckDOM.h:
        (WebCore::DOMJIT::TypeChecker<DocumentFragment>::branchIfFail):
        * domjit/DOMJITHelpers.h:
        (WebCore::DOMJIT::branchIfDocumentFragment):
        (WebCore::DOMJIT::branchIfNotDocumentFragment):

2016-11-04  Joanmarie Diggs  <jdiggs@igalia.com>

        AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash

Lines 28-32 a/Source/WebCore/dom/NonElementParentNode.idl_sec1
- a/Source/WebCore/dom/NonElementParentNode.idl -1 / +1 lines
28	[	28	[
29	NoInterfaceObject,	29	NoInterfaceObject,
30	] interface NonElementParentNode {	30	] interface NonElementParentNode {
31	Element getElementById([RequiresExistingAtomicString] DOMString elementId);	31	[DOMJIT=ReadDOM] Element? getElementById([RequiresExistingAtomicString] DOMString elementId);
32	};	32	};

Lines 58-63 struct TypeChecker<Document> { a/Source/WebCore/domjit/DOMJITCheckDOM.h_sec1
- a/Source/WebCore/domjit/DOMJITCheckDOM.h +8 lines
58	};	58	};
59		59
60	template<>	60	template<>
		61	struct TypeChecker<DocumentFragment> {
		62	static CCallHelpers::Jump branchIfFail(CCallHelpers& jit, GPRReg dom)
		63	{
		64	return DOMJIT::branchIfNotDocumentFragment(jit, dom);
		65	}
		66	};
		67
		68	template<>
61	struct TypeChecker<Event> {	69	struct TypeChecker<Event> {
62	static CCallHelpers::Jump branchIfFail(CCallHelpers& jit, GPRReg dom)	70	static CCallHelpers::Jump branchIfFail(CCallHelpers& jit, GPRReg dom)
63	{	71	{



        CCallHelpers::TrustedImm32(JSC::JSType(JSElementType)));
}

inline CCallHelpers::Jump branchIfDocumentFragment(CCallHelpers& jit, GPRReg target)
{
    return jit.branchIfType(target, JSC::JSType(JSDocumentFragmentNodeType));
}

inline CCallHelpers::Jump branchIfNotDocumentFragment(CCallHelpers& jit, GPRReg target)
{
    return jit.branchIfNotType(target, JSC::JSType(JSDocumentFragmentNodeType));
}

inline CCallHelpers::Jump branchIfDocumentWrapper(CCallHelpers& jit, GPRReg target)
{
    return jit.branchIfType(target, JSC::JSType(JSDocumentWrapperType));



2016-11-04  Yusuke Suzuki  <utatane.tea@gmail.com>

        [DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById
        https://bugs.webkit.org/show_bug.cgi?id=164356

        Reviewed by Filip Pizlo.

        * js/dom/domjit-function-get-element-by-id-changed-expected.txt: Added.
        * js/dom/domjit-function-get-element-by-id-changed.html: Added.
        * js/dom/domjit-function-get-element-by-id-expected.txt: Added.
        * js/dom/domjit-function-get-element-by-id-licm-expected.txt: Added.
        * js/dom/domjit-function-get-element-by-id-licm.html: Added.
        * js/dom/domjit-function-get-element-by-id.html: Added.

2016-11-04  Joanmarie Diggs  <jdiggs@igalia.com>

        AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash



Test LICM-ed DOMJIT function getElementById will work correctly when the function is replaced.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS successfullyParsed is true

TEST COMPLETE




<!DOCTYPE html>
<html>
<head>
<script src="../../resources/js-test-pre.js"></script>
</head>
<body>
<div id="description"></div>
<div id="console"></div>

<div id="parentNode">
<div id="previousSibling"></div><div id="target"><div id="firstChild"></div><div id="lastChild"></div></div><div id="nextSibling"></div>
</div>

<script>
description('Test LICM-ed DOMJIT function getElementById will work correctly when the function is replaced.');

function test(flag) {
    var ret = 0;
    var text = document.createTextNode('Cocoa');
    for (var i = 0; i < 1e4; ++i) {
        ret = document.getElementById("target").nodeType;
        ret = document.getElementById("target").nodeType;
        if (i === 100 && flag)
            document.getElementById = function (id) { return text; };
        ret = document.getElementById("target").nodeType;
        ret = document.getElementById("target").nodeType;
    }
    return ret;
}
var result;
(function () {
    for (var i = 0; i < 100; ++i) {
        result = test(false);
        shouldBe(`result`, `1`, true);
    }
    var original = document.getElementById;
    result = test(true);
    document.getElementById = original;
    shouldBe(`result`, `Node.TEXT_NODE`, true);
}());
</script>
<script src="../../resources/js-test-post.js"></script>
</body>
</html>



Test DOMJIT function getElementById will be LICM-ed.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS successfullyParsed is true

TEST COMPLETE




Test DOMJIT function getElementById will be LICM-ed.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS successfullyParsed is true

TEST COMPLETE




<!DOCTYPE html>
<html>
<head>
<script src="../../resources/js-test-pre.js"></script>
</head>
<body>
<div id="description"></div>
<div id="console"></div>

<div id="parentNode">
<div id="previousSibling"></div><div id="target"><div id="firstChild"></div><div id="lastChild"></div></div><div id="nextSibling"></div>
</div>

<script>
description('Test DOMJIT function getElementById will be LICM-ed.');

function test() {
    var ret = 0;
    for (var i = 0; i < 1e4; ++i) {
        ret = document.getElementById("target").nodeType;
        ret = document.getElementById("target").nodeType;
        ret = document.getElementById("target").nodeType;
        ret = document.getElementById("target").nodeType;
    }
    return ret;
}
var result;
(function () {
    for (var i = 0; i < 100; ++i) {
        result = test();
        shouldBe(`result`, `1`, true);
    }
}());
</script>
<script src="../../resources/js-test-post.js"></script>
</body>
</html>



<!DOCTYPE html>
<html>
<head>
<script src="../../resources/js-test-pre.js"></script>
</head>
<body>
<div id="description"></div>
<div id="console"></div>

<div id="parentNode">
<div id="previousSibling"></div><div id="target"><div id="firstChild"></div><div id="lastChild"></div></div><div id="nextSibling"></div>
</div>

<script>
description('Test DOMJIT function getElementById will be LICM-ed.');

function test(doc, id) {
    var ret = 0;
    for (var i = 0; i < 1e4; ++i) {
        ret = doc.getElementById(id).nodeType;
    }
    return ret;
}
var result;
(function () {
    var fragment = document.createDocumentFragment();
    var div = document.createElement('div');
    div.setAttribute('id', 'target2');
    fragment.appendChild(div);
    for (var i = 0; i < 100; ++i) {
        result = test(document, 'target');
        shouldBe(`result`, `1`, true);
        result = test(fragment, 'target2');
        shouldBe(`result`, `1`, true);
    }
}());
</script>
<script src="../../resources/js-test-post.js"></script>
</body>
</html>

Return to Bug 164356

Lines 152-157 inline CCallHelpers::Jump branchIfNotElement(CCallHelpers& jit, GPRReg target) a/Source/WebCore/domjit/DOMJITHelpers.h_sec1
- a/Source/WebCore/domjit/DOMJITHelpers.h +10 lines
152	CCallHelpers::TrustedImm32(JSC::JSType(JSElementType)));	152	CCallHelpers::TrustedImm32(JSC::JSType(JSElementType)));
153	}	153	}
154		154
		155	inline CCallHelpers::Jump branchIfDocumentFragment(CCallHelpers& jit, GPRReg target)
		156	{
		157	return jit.branchIfType(target, JSC::JSType(JSDocumentFragmentNodeType));
		158	}
		159
		160	inline CCallHelpers::Jump branchIfNotDocumentFragment(CCallHelpers& jit, GPRReg target)
		161	{
		162	return jit.branchIfNotType(target, JSC::JSType(JSDocumentFragmentNodeType));
		163	}
		164
155	inline CCallHelpers::Jump branchIfDocumentWrapper(CCallHelpers& jit, GPRReg target)	165	inline CCallHelpers::Jump branchIfDocumentWrapper(CCallHelpers& jit, GPRReg target)
156	{	166	{
157	return jit.branchIfType(target, JSC::JSType(JSDocumentWrapperType));	167	return jit.branchIfType(target, JSC::JSType(JSDocumentWrapperType));

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +47 lines
		1	2016-11-04 Yusuke Suzuki <utatane.tea@gmail.com>
		2
		3	[DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById
		4	https://bugs.webkit.org/show_bug.cgi?id=164356
		5
		6	Reviewed by Filip Pizlo.
		7
		8	This patch implements DOMJIT::Signature annotation for getElementById.
		9	Since getElementById is also implemented in DocumentFragment, we implement
		10	the branchIfDocumentFragment/branchIfNotDocumentFragment for that.
		11
		12	In dromaeo, we have a test like this.
		13
		14	test( "getElementById", function(){
		15	for ( var i = 0; i < num * 30; i++ ) {
		16	ret = document.getElementById("testA" + num).nodeType;
		17	ret = document.getElementById("testB" + num).nodeType;
		18	ret = document.getElementById("testC" + num).nodeType;
		19	ret = document.getElementById("testD" + num).nodeType;
		20	ret = document.getElementById("testE" + num).nodeType;
		21	ret = document.getElementById("testF" + num).nodeType;
		22	}
		23	});
		24
		25	In the above test, JSC already knows the following things.
		26
		27	1. Since nodeType is now handled as CallDOMGetter, we know that it is pure.
		28	2. getElementById look up becomes PureGetById since document is impure object. But it is kept as PureGetById. So it does not write DOMState.
		29	3. `"testA" + num` will be converted to constant string.
		30	4. CallDOM for getElementById said it just reads(DOMState:DOM). And it saids that it returns the same value as long as DOMState is not clobbered.
		31	5. CheckCell leading CallDOM ensures the inlined getElementById node. (CallDOM node).
		32
		33	The key thing is that no node clobbers DOMState during the loop. So CallDOM & CallDOMGetter can be hoisted.
		34	This improves dom-query significantly. Dromaeo dom-query getElementById becomes 40x faster (247796 v.s. 6197).
		35	Dromaeo dom-query getElementById (not in document) becomes 89x faster (630317.8 v.s. 7066.).
		36
		37	Tests: js/dom/domjit-function-get-element-by-id-changed.html
		38	js/dom/domjit-function-get-element-by-id-licm.html
		39	js/dom/domjit-function-get-element-by-id.html
		40
		41	* dom/NonElementParentNode.idl:
		42	* domjit/DOMJITCheckDOM.h:
		43	(WebCore::DOMJIT::TypeChecker<DocumentFragment>::branchIfFail):
		44	* domjit/DOMJITHelpers.h:
		45	(WebCore::DOMJIT::branchIfDocumentFragment):
		46	(WebCore::DOMJIT::branchIfNotDocumentFragment):
		47
1	2016-11-04 Joanmarie Diggs <jdiggs@igalia.com>	48	2016-11-04 Joanmarie Diggs <jdiggs@igalia.com>
2		49
3	AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash	50	AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +14 lines
		1	2016-11-04 Yusuke Suzuki <utatane.tea@gmail.com>
		2
		3	[DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById
		4	https://bugs.webkit.org/show_bug.cgi?id=164356
		5
		6	Reviewed by Filip Pizlo.
		7
		8	* js/dom/domjit-function-get-element-by-id-changed-expected.txt: Added.
		9	* js/dom/domjit-function-get-element-by-id-changed.html: Added.
		10	* js/dom/domjit-function-get-element-by-id-expected.txt: Added.
		11	* js/dom/domjit-function-get-element-by-id-licm-expected.txt: Added.
		12	* js/dom/domjit-function-get-element-by-id-licm.html: Added.
		13	* js/dom/domjit-function-get-element-by-id.html: Added.
		14
1	2016-11-04 Joanmarie Diggs <jdiggs@igalia.com>	15	2016-11-04 Joanmarie Diggs <jdiggs@igalia.com>
2		16
3	AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash	17	AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash

Line 0 a/LayoutTests/js/dom/domjit-function-get-element-by-id-changed-expected.txt_sec1
- a/LayoutTests/js/dom/domjit-function-get-element-by-id-changed-expected.txt +8 lines
	1	Test LICM-ed DOMJIT function getElementById will work correctly when the function is replaced.
	2
	3	On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
	4
	5	PASS successfullyParsed is true
	6
	7	TEST COMPLETE
	8

Line 0 a/LayoutTests/js/dom/domjit-function-get-element-by-id-changed.html_sec1
- a/LayoutTests/js/dom/domjit-function-get-element-by-id-changed.html +44 lines
	1	<!DOCTYPE html>
	2	<html>
	3	<head>
	4	<script src="../../resources/js-test-pre.js"></script>
	5	</head>
	6	<body>
	7	<div id="description"></div>
	8	<div id="console"></div>
	9
	10	<div id="parentNode">
	11	<div id="previousSibling"></div><div id="target"><div id="firstChild"></div><div id="lastChild"></div></div><div id="nextSibling"></div>
	12	</div>
	13
	14	<script>
	15	description('Test LICM-ed DOMJIT function getElementById will work correctly when the function is replaced.');
	16
	17	function test(flag) {
	18	var ret = 0;
	19	var text = document.createTextNode('Cocoa');
	20	for (var i = 0; i < 1e4; ++i) {
	21	ret = document.getElementById("target").nodeType;
	22	ret = document.getElementById("target").nodeType;
	23	if (i === 100 && flag)
	24	document.getElementById = function (id) { return text; };
	25	ret = document.getElementById("target").nodeType;
	26	ret = document.getElementById("target").nodeType;
	27	}
	28	return ret;
	29	}
	30	var result;
	31	(function () {
	32	for (var i = 0; i < 100; ++i) {
	33	result = test(false);
	34	shouldBe(`result`, `1`, true);
	35	}
	36	var original = document.getElementById;
	37	result = test(true);
	38	document.getElementById = original;
	39	shouldBe(`result`, `Node.TEXT_NODE`, true);
	40	}());
	41	</script>
	42	<script src="../../resources/js-test-post.js"></script>
	43	</body>
	44	</html>

Line 0 a/LayoutTests/js/dom/domjit-function-get-element-by-id-expected.txt_sec1
- a/LayoutTests/js/dom/domjit-function-get-element-by-id-expected.txt +8 lines
	1	Test DOMJIT function getElementById will be LICM-ed.
	2
	3	On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
	4
	5	PASS successfullyParsed is true
	6
	7	TEST COMPLETE
	8