Log Message
Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists https://bugs.webkit.org/show_bug.cgi?id=146141
Patch by Joseph Pecoraro <[email protected]> on 2015-06-19 Reviewed by Timothy Hatcher. Source/_javascript_Core: * inspector/InjectedScriptSource.js: (InjectedScript.prototype._propertyDescriptors): Avoid calling getOwnPropertyNames/Symbols on very large lists. Instead just generate property descriptors for the first 100 indexes. Note this would behave poorly for sparse arrays with a length > 100, but general support for lists with more than 100 elements is poor. See: <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees Source/WebInspectorUI: * UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js: (WebInspector._javascript_RuntimeCompletionProvider.prototype.completionControllerCompletionsNeeded): When building completions for a large array, instead of building a list of all the indices just send a single number back to the frontend and let it generate a list for completion. We should do even better and avoid building a lookup table here for completion of an index. LayoutTests: * inspector/model/remote-object-expected.txt: * inspector/model/remote-object.html: There were existing progressions (0 => -1). There were existing regressions (RegExp properties moving to prototype with descriptors on the prototype). Add a test for a very large list that would have caused out of memory / hang.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/inspector/model/remote-object-expected.txt
- trunk/LayoutTests/inspector/model/remote-object.html
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/inspector/InjectedScriptSource.js
- trunk/Source/WebInspectorUI/ChangeLog
- trunk/Source/WebInspectorUI/UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js
Diff
Modified: trunk/LayoutTests/ChangeLog (185740 => 185741)
--- trunk/LayoutTests/ChangeLog 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/LayoutTests/ChangeLog 2015-06-19 08:05:54 UTC (rev 185741)
@@ -1,3 +1,16 @@
+2015-06-19 Joseph Pecoraro <[email protected]>
+
+ Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
+ https://bugs.webkit.org/show_bug.cgi?id=146141
+
+ Reviewed by Timothy Hatcher.
+
+ * inspector/model/remote-object-expected.txt:
+ * inspector/model/remote-object.html:
+ There were existing progressions (0 => -1).
+ There were existing regressions (RegExp properties moving to prototype with descriptors on the prototype).
+ Add a test for a very large list that would have caused out of memory / hang.
+
2015-06-18 Gyuyoung Kim <[email protected]>
[EFL] Unreviewed, gardening on June 19th.
Modified: trunk/LayoutTests/inspector/model/remote-object-expected.txt (185740 => 185741)
--- trunk/LayoutTests/inspector/model/remote-object-expected.txt 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/LayoutTests/inspector/model/remote-object-expected.txt 2015-06-19 08:05:54 UTC (rev 185741)
@@ -273,26 +273,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": " "
}
],
"_entries": null
@@ -317,26 +297,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": "(?:)"
}
],
"_entries": null
@@ -361,26 +321,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": "^r(e)g[e]{1,}x+"
}
],
"_entries": null
@@ -405,26 +345,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "true"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "true"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": "^r(e)g[e]{1,}x+"
}
],
"_entries": null
@@ -449,26 +369,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": "(?:)"
}
],
"_entries": null
@@ -493,26 +393,6 @@
"_name": "lastIndex",
"_type": "number",
"_value": "0"
- },
- {
- "_name": "global",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "ignoreCase",
- "_type": "boolean",
- "_value": "true"
- },
- {
- "_name": "multiline",
- "_type": "boolean",
- "_value": "false"
- },
- {
- "_name": "source",
- "_type": "string",
- "_value": "test"
}
],
"_entries": null
@@ -563,7 +443,7 @@
{
"_name": "1",
"_type": "number",
- "_value": "0"
+ "_value": "-0"
},
{
"_name": "2",
@@ -1199,6 +1079,77 @@
}
-----------------------------------------------------
+_expression_: var buffer = new ArrayBuffer(10000000); var int8View = new Int8Array(buffer); int8View
+{
+ "_type": "object",
+ "_subtype": "array",
+ "_objectId": "<filtered>",
+ "_description": "Int8Array",
+ "_size": 10000000,
+ "_preview": {
+ "_type": "object",
+ "_subtype": "array",
+ "_description": "Int8Array",
+ "_lossless": false,
+ "_overflow": true,
+ "_size": 10000000,
+ "_properties": [
+ {
+ "_name": "0",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "1",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "2",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "3",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "4",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "5",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "6",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "7",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "8",
+ "_type": "number",
+ "_value": "0"
+ },
+ {
+ "_name": "9",
+ "_type": "number",
+ "_value": "0"
+ }
+ ],
+ "_entries": null
+ }
+}
+
+-----------------------------------------------------
_expression_: ({})
{
"_type": "object",
@@ -1261,7 +1212,7 @@
{
"_name": "c",
"_type": "number",
- "_value": "0"
+ "_value": "-0"
}
],
"_entries": null
Modified: trunk/LayoutTests/inspector/model/remote-object.html (185740 => 185741)
--- trunk/LayoutTests/inspector/model/remote-object.html 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/LayoutTests/inspector/model/remote-object.html 2015-06-19 08:05:54 UTC (rev 185741)
@@ -87,6 +87,7 @@
{_expression_: "a = null; (function() { a = arguments; })(1, '2', /3/); a"},
{_expression_: "new Int32Array(new ArrayBuffer(16))"},
{_expression_: "var intArray = new Int32Array(new ArrayBuffer(16)); for (var i = 0; i < intArray.length; ++i) intArray[i] = i; intArray"},
+ {_expression_: "var buffer = new ArrayBuffer(10000000); var int8View = new Int8Array(buffer); int8View"}, // 10000000 elements.
// Object
{_expression_: "({})"},
Modified: trunk/Source/_javascript_Core/ChangeLog (185740 => 185741)
--- trunk/Source/_javascript_Core/ChangeLog 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-06-19 08:05:54 UTC (rev 185741)
@@ -1,3 +1,18 @@
+2015-06-19 Joseph Pecoraro <[email protected]>
+
+ Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
+ https://bugs.webkit.org/show_bug.cgi?id=146141
+
+ Reviewed by Timothy Hatcher.
+
+ * inspector/InjectedScriptSource.js:
+ (InjectedScript.prototype._propertyDescriptors):
+ Avoid calling getOwnPropertyNames/Symbols on very large lists. Instead
+ just generate property descriptors for the first 100 indexes. Note
+ this would behave poorly for sparse arrays with a length > 100, but
+ general support for lists with more than 100 elements is poor. See:
+ <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
+
2015-06-18 Yusuke Suzuki <[email protected]>
[DFG] Avoid OSR exit in the middle of string concatenation
Modified: trunk/Source/_javascript_Core/inspector/InjectedScriptSource.js (185740 => 185741)
--- trunk/Source/_javascript_Core/inspector/InjectedScriptSource.js 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/Source/_javascript_Core/inspector/InjectedScriptSource.js 2015-06-19 08:05:54 UTC (rev 185741)
@@ -698,12 +698,34 @@
}
}
- // Iterate prototype chain.
+ function arrayIndexPropertyNames(o, length)
+ {
+ var array = new Array(length);
+ for (var i = 0; i < length; ++i) {
+ if (i in o)
+ array.push("" + i);
+ }
+ return array;
+ }
+
+ // FIXME: <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
+ // For array types with a large length we attempt to skip getOwnPropertyNames and instead just sublist of indexes.
+ var isArrayTypeWithLargeLength = false;
+ try {
+ isArrayTypeWithLargeLength = injectedScript._subtype(object) === "array" && isFinite(object.length) && object.length > 100;
+ } catch(e) {}
+
for (var o = object; this._isDefined(o); o = o.__proto__) {
var isOwnProperty = o === object;
- processProperties(o, Object.getOwnPropertyNames(o), isOwnProperty);
- if (Object.getOwnPropertySymbols)
- processProperties(o, Object.getOwnPropertySymbols(o), isOwnProperty);
+
+ if (isArrayTypeWithLargeLength && isOwnProperty)
+ processProperties(o, arrayIndexPropertyNames(o, 100), isOwnProperty);
+ else {
+ processProperties(o, Object.getOwnPropertyNames(o), isOwnProperty);
+ if (Object.getOwnPropertySymbols)
+ processProperties(o, Object.getOwnPropertySymbols(o), isOwnProperty);
+ }
+
if (collectionMode === InjectedScript.CollectionMode.OwnProperties)
break;
}
Modified: trunk/Source/WebInspectorUI/ChangeLog (185740 => 185741)
--- trunk/Source/WebInspectorUI/ChangeLog 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/Source/WebInspectorUI/ChangeLog 2015-06-19 08:05:54 UTC (rev 185741)
@@ -1,3 +1,16 @@
+2015-06-19 Joseph Pecoraro <[email protected]>
+
+ Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
+ https://bugs.webkit.org/show_bug.cgi?id=146141
+
+ Reviewed by Timothy Hatcher.
+
+ * UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js:
+ (WebInspector._javascript_RuntimeCompletionProvider.prototype.completionControllerCompletionsNeeded):
+ When building completions for a large array, instead of building a list of all the indices
+ just send a single number back to the frontend and let it generate a list for completion.
+ We should do even better and avoid building a lookup table here for completion of an index.
+
2015-06-18 Matt Baker <[email protected]>
Web Inspector: Rendering Frames timeline selection should snap to frame boundaries
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js (185740 => 185741)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js 2015-06-19 07:58:54 UTC (rev 185740)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/_javascript_RuntimeCompletionProvider.js 2015-06-19 08:05:54 UTC (rev 185741)
@@ -125,6 +125,34 @@
return;
}
+ function getArrayCompletions(primitiveType)
+ {
+ var array = this;
+ var arrayLength;
+
+ var resultSet = {};
+ for (var o = array; o; o = o.__proto__) {
+ try {
+ if (o === array && o.length) {
+ // If the array type has a length, don't include a list of all the indexes.
+ // Include it at the end and the frontend can build the list.
+ arrayLength = o.length;
+ } else {
+ var names = Object.getOwnPropertyNames(o);
+ for (var i = 0; i < names.length; ++i)
+ resultSet[names[i]] = true;
+ }
+ } catch (e) {
+ // Ignore
+ }
+ }
+
+ if (arrayLength)
+ resultSet["length"] = arrayLength;
+
+ return resultSet;
+ }
+
function getCompletions(primitiveType)
{
var object;
@@ -151,7 +179,9 @@
return resultSet;
}
- if (result.type === "object" || result.type === "function")
+ if (result.subtype === "array")
+ result.callFunctionJSON(getArrayCompletions, undefined, receivedArrayPropertyNames.bind(this));
+ else if (result.type === "object" || result.type === "function")
result.callFunctionJSON(getCompletions, undefined, receivedPropertyNames.bind(this));
else if (result.type === "string" || result.type === "number" || result.type === "boolean")
WebInspector.runtimeManager.evaluateInInspectedWindow("(" + getCompletions + ")(\"" + result.type + "\")", "completion", false, true, true, false, false, receivedPropertyNamesFromEvaluate.bind(this));
@@ -164,6 +194,20 @@
receivedPropertyNames.call(this, result && !wasThrown ? result.value : null);
}
+ function receivedArrayPropertyNames(propertyNames)
+ {
+ // FIXME: <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
+ // If there was an array like object, we generate autocompletion up to 1000 indexes, but this should
+ // handle a list with arbitrary length.
+ if (propertyNames && typeof propertyNames.length === "number") {
+ var max = Math.min(propertyNames.length, 1000);
+ for (var i = 0; i < max; ++i)
+ propertyNames[i] = true;
+ }
+
+ receivedPropertyNames.call(this, propertyNames);
+ }
+
function receivedPropertyNames(propertyNames)
{
propertyNames = propertyNames || {};
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
