Title: [149834] trunk
- Revision
- 149834
- Author
- [email protected]
- Date
- 2013-05-09 12:38:23 -0700 (Thu, 09 May 2013)
Log Message
DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes
https://bugs.webkit.org/show_bug.cgi?id=115805
Source/_javascript_Core:
Reviewed by Geoffrey Garen.
It checks the observed ArrayModes to see if we have seen any ArrayWith* first. If so, it assumes it's
an Array::Array, even if we've also observed any NonArrayWith* in the ArrayProfile. This leads to the
code generated by jumpSlowForUnwantedArrayMode to check the indexing type against (shape | IsArray)
instead of just shape, which can cause us to exit a lot in the case that we saw a NonArray.
To fix this we need to add a case that checks for both ArrayWith* and NonArrayWith* cases first, which
should then use Array::PossiblyArray, then do the checks we were already doing.
* bytecode/ArrayProfile.h:
(JSC::hasSeenArray):
(JSC::hasSeenNonArray):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::fromObserved):
LayoutTests:
Added regression test for array access over polymorphic array vs. non-array indexing types.
With the fix, we get 3.666x faster on this microbenchmark.
Reviewed by Geoffrey Garen.
* fast/js/regress/array-nonarray-polymorphic-access-expected.txt: Added.
* fast/js/regress/array-nonarray-polymorphic-access.html: Added.
* fast/js/regress/script-tests/array-nonarray-polymorphic-access.js: Added.
(f):
(run):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (149833 => 149834)
--- trunk/LayoutTests/ChangeLog 2013-05-09 19:24:05 UTC (rev 149833)
+++ trunk/LayoutTests/ChangeLog 2013-05-09 19:38:23 UTC (rev 149834)
@@ -1,3 +1,37 @@
+2013-05-08 Mark Hahnenberg <[email protected]>
+
+ DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes
+ https://bugs.webkit.org/show_bug.cgi?id=115805
+
+ Added regression test for array access over polymorphic array vs. non-array indexing types.
+ With the fix, we get 3.666x faster on this microbenchmark.
+
+ Reviewed by Geoffrey Garen.
+
+ * fast/js/regress/array-nonarray-polymorphic-access-expected.txt: Added.
+ * fast/js/regress/array-nonarray-polymorphic-access.html: Added.
+ * fast/js/regress/script-tests/array-nonarray-polymorphic-access.js: Added.
+ (f):
+ (run):
+
+<<<<<<< .mine
+2013-05-08 Mark Hahnenberg <[email protected]>
+
+ DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes
+ https://bugs.webkit.org/show_bug.cgi?id=115805
+
+ Reviewed by Geoffrey Garen.
+
+ Added regression test for array access over polymorphic array vs. non-array indexing types.
+ With the fix, we get 3.666x faster on this microbenchmark.
+
+ * fast/js/regress/array-nonarray-polymorphic-access-expected.txt: Added.
+ * fast/js/regress/array-nonarray-polymorphic-access.html: Added.
+ * fast/js/regress/script-tests/array-nonarray-polymorhpic-access.js: Added.
+ (f):
+ (run):
+
+=======
2013-05-09 Radu Stavila <[email protected]>
[CSSRegions] Change fast/regions/webkit-flow-inlines-inside-regions-bounds[-vertical] tests to make them platform independent
@@ -96,6 +130,7 @@
* platform/qt/fast/js/global-constructors-attributes-expected.txt: Added after r149758.
+>>>>>>> .r149833
2013-05-08 Seokju Kwon <[email protected]>
[EFL] Get rid of duplicate entry line in TestExpectations
Added: trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access-expected.txt (0 => 149834)
--- trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access-expected.txt 2013-05-09 19:38:23 UTC (rev 149834)
@@ -0,0 +1,10 @@
+JSRegress/array-nonarray-polymorphic-access
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access.html (0 => 149834)
--- trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access.html (rev 0)
+++ trunk/LayoutTests/fast/js/regress/array-nonarray-polymorphic-access.html 2013-05-09 19:38:23 UTC (rev 149834)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/regress/script-tests/array-nonarray-polymorhpic-access.js (0 => 149834)
--- trunk/LayoutTests/fast/js/regress/script-tests/array-nonarray-polymorhpic-access.js (rev 0)
+++ trunk/LayoutTests/fast/js/regress/script-tests/array-nonarray-polymorhpic-access.js 2013-05-09 19:38:23 UTC (rev 149834)
@@ -0,0 +1,33 @@
+var f = function(a) {
+ var sum = 0;
+ for (var i = 0; i < a.length; i++) {
+ sum += a[i];
+ }
+ return sum;
+};
+
+var run = function() {
+ var o1 = [];
+ for (var i = 0; i < 100; i++) {
+ o1[i] = i;
+ }
+
+ var o2 = {};
+ for (var i = 0; i < o1.length; i++) {
+ o2[i] = o1[i];
+ }
+ o2.length = o1.length;
+
+ var sum = 0;
+ for (var i = 0; i < 100000; i++) {
+ if (i % 2 === 0)
+ sum += f(o1);
+ else
+ sum += f(o2);
+ }
+ return sum;
+};
+
+var result = run();
+if (result !== 495000000)
+ throw "Bad result";
Modified: trunk/Source/_javascript_Core/ChangeLog (149833 => 149834)
--- trunk/Source/_javascript_Core/ChangeLog 2013-05-09 19:24:05 UTC (rev 149833)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-05-09 19:38:23 UTC (rev 149834)
@@ -1,3 +1,24 @@
+2013-05-08 Mark Hahnenberg <[email protected]>
+
+ DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes
+ https://bugs.webkit.org/show_bug.cgi?id=115805
+
+ Reviewed by Geoffrey Garen.
+
+ It checks the observed ArrayModes to see if we have seen any ArrayWith* first. If so, it assumes it's
+ an Array::Array, even if we've also observed any NonArrayWith* in the ArrayProfile. This leads to the
+ code generated by jumpSlowForUnwantedArrayMode to check the indexing type against (shape | IsArray)
+ instead of just shape, which can cause us to exit a lot in the case that we saw a NonArray.
+
+ To fix this we need to add a case that checks for both ArrayWith* and NonArrayWith* cases first, which
+ should then use Array::PossiblyArray, then do the checks we were already doing.
+
+ * bytecode/ArrayProfile.h:
+ (JSC::hasSeenArray):
+ (JSC::hasSeenNonArray):
+ * dfg/DFGArrayMode.cpp:
+ (JSC::DFG::ArrayMode::fromObserved):
+
2013-05-09 Joe Mason <[email protected]>
[BlackBerry] Set up logging buffer on start of jsc executable
Modified: trunk/Source/_javascript_Core/bytecode/ArrayProfile.h (149833 => 149834)
--- trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2013-05-09 19:24:05 UTC (rev 149833)
+++ trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2013-05-09 19:38:23 UTC (rev 149834)
@@ -115,6 +115,16 @@
return arrayModesInclude(arrayModes, Int32Shape);
}
+inline bool hasSeenArray(ArrayModes arrayModes)
+{
+ return arrayModes & ALL_ARRAY_ARRAY_MODES;
+}
+
+inline bool hasSeenNonArray(ArrayModes arrayModes)
+{
+ return arrayModes & ALL_NON_ARRAY_ARRAY_MODES;
+}
+
class ArrayProfile {
public:
ArrayProfile()
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp (149833 => 149834)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2013-05-09 19:24:05 UTC (rev 149833)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2013-05-09 19:38:23 UTC (rev 149834)
@@ -112,9 +112,11 @@
else
type = Array::Undecided;
- if (observed & (asArrayModes(ArrayWithUndecided) | asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) | asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage)))
+ if (hasSeenArray(observed) && hasSeenNonArray(observed))
+ arrayClass = Array::PossiblyArray;
+ else if (hasSeenArray(observed))
arrayClass = Array::Array;
- else if (observed & (asArrayModes(NonArray) | asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) | asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage)))
+ else if (hasSeenNonArray(observed))
arrayClass = Array::NonArray;
else
arrayClass = Array::PossiblyArray;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
