⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 185741 in webkit


Ignore:
Timestamp:
Jun 19, 2015, 1:05:54 AM (11 years ago)
Author:
commit-queue@webkit.org
Message:

Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
​https://bugs.webkit.org/show_bug.cgi?id=146141

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2015-06-19
Reviewed by Timothy Hatcher.

Source/JavaScriptCore:

  • 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/JavaScriptRuntimeCompletionProvider.js:

(WebInspector.JavaScriptRuntimeCompletionProvider.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.

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r185735 r185741  
     12015-06-19  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
     4        https://bugs.webkit.org/show_bug.cgi?id=146141
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        * inspector/model/remote-object-expected.txt:
     9        * inspector/model/remote-object.html:
     10        There were existing progressions (0 => -1).
     11        There were existing regressions (RegExp properties moving to prototype with descriptors on the prototype).
     12        Add a test for a very large list that would have caused out of memory / hang.
     13
    1142015-06-18  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
    215

  • trunk/LayoutTests/inspector/model/remote-object-expected.txt

    r185237 r185741  
    274274        "_type": "number",
    275275        "_value": "0"
    276       },
    277       {
    278         "_name": "global",
    279         "_type": "boolean",
    280         "_value": "false"
    281       },
    282       {
    283         "_name": "ignoreCase",
    284         "_type": "boolean",
    285         "_value": "false"
    286       },
    287       {
    288         "_name": "multiline",
    289         "_type": "boolean",
    290         "_value": "false"
    291       },
    292       {
    293         "_name": "source",
    294         "_type": "string",
    295         "_value": " "
    296276      }
    297277    ],
    … …  
    318298        "_type": "number",
    319299        "_value": "0"
    320       },
    321       {
    322         "_name": "global",
    323         "_type": "boolean",
    324         "_value": "false"
    325       },
    326       {
    327         "_name": "ignoreCase",
    328         "_type": "boolean",
    329         "_value": "false"
    330       },
    331       {
    332         "_name": "multiline",
    333         "_type": "boolean",
    334         "_value": "false"
    335       },
    336       {
    337         "_name": "source",
    338         "_type": "string",
    339         "_value": "(?:)"
    340300      }
    341301    ],
    … …  
    362322        "_type": "number",
    363323        "_value": "0"
    364       },
    365       {
    366         "_name": "global",
    367         "_type": "boolean",
    368         "_value": "false"
    369       },
    370       {
    371         "_name": "ignoreCase",
    372         "_type": "boolean",
    373         "_value": "false"
    374       },
    375       {
    376         "_name": "multiline",
    377         "_type": "boolean",
    378         "_value": "false"
    379       },
    380       {
    381         "_name": "source",
    382         "_type": "string",
    383         "_value": "^r(e)g[e]{1,}x+"
    384324      }
    385325    ],
    … …  
    406346        "_type": "number",
    407347        "_value": "0"
    408       },
    409       {
    410         "_name": "global",
    411         "_type": "boolean",
    412         "_value": "true"
    413       },
    414       {
    415         "_name": "ignoreCase",
    416         "_type": "boolean",
    417         "_value": "true"
    418       },
    419       {
    420         "_name": "multiline",
    421         "_type": "boolean",
    422         "_value": "false"
    423       },
    424       {
    425         "_name": "source",
    426         "_type": "string",
    427         "_value": "^r(e)g[e]{1,}x+"
    428348      }
    429349    ],
    … …  
    450370        "_type": "number",
    451371        "_value": "0"
    452       },
    453       {
    454         "_name": "global",
    455         "_type": "boolean",
    456         "_value": "false"
    457       },
    458       {
    459         "_name": "ignoreCase",
    460         "_type": "boolean",
    461         "_value": "false"
    462       },
    463       {
    464         "_name": "multiline",
    465         "_type": "boolean",
    466         "_value": "false"
    467       },
    468       {
    469         "_name": "source",
    470         "_type": "string",
    471         "_value": "(?:)"
    472372      }
    473373    ],
    … …  
    494394        "_type": "number",
    495395        "_value": "0"
    496       },
    497       {
    498         "_name": "global",
    499         "_type": "boolean",
    500         "_value": "false"
    501       },
    502       {
    503         "_name": "ignoreCase",
    504         "_type": "boolean",
    505         "_value": "true"
    506       },
    507       {
    508         "_name": "multiline",
    509         "_type": "boolean",
    510         "_value": "false"
    511       },
    512       {
    513         "_name": "source",
    514         "_type": "string",
    515         "_value": "test"
    516396      }
    517397    ],
    … …  
    564444        "_name": "1",
    565445        "_type": "number",
    566         "_value": "0"
     446        "_value": "-0"
    567447      },
    568448      {
    … …  
    12001080
    12011081-----------------------------------------------------
     1082EXPRESSION: var buffer = new ArrayBuffer(10000000); var int8View = new Int8Array(buffer); int8View
     1083{
     1084  "_type": "object",
     1085  "_subtype": "array",
     1086  "_objectId": "<filtered>",
     1087  "_description": "Int8Array",
     1088  "_size": 10000000,
     1089  "_preview": {
     1090    "_type": "object",
     1091    "_subtype": "array",
     1092    "_description": "Int8Array",
     1093    "_lossless": false,
     1094    "_overflow": true,
     1095    "_size": 10000000,
     1096    "_properties": [
     1097      {
     1098        "_name": "0",
     1099        "_type": "number",
     1100        "_value": "0"
     1101      },
     1102      {
     1103        "_name": "1",
     1104        "_type": "number",
     1105        "_value": "0"
     1106      },
     1107      {
     1108        "_name": "2",
     1109        "_type": "number",
     1110        "_value": "0"
     1111      },
     1112      {
     1113        "_name": "3",
     1114        "_type": "number",
     1115        "_value": "0"
     1116      },
     1117      {
     1118        "_name": "4",
     1119        "_type": "number",
     1120        "_value": "0"
     1121      },
     1122      {
     1123        "_name": "5",
     1124        "_type": "number",
     1125        "_value": "0"
     1126      },
     1127      {
     1128        "_name": "6",
     1129        "_type": "number",
     1130        "_value": "0"
     1131      },
     1132      {
     1133        "_name": "7",
     1134        "_type": "number",
     1135        "_value": "0"
     1136      },
     1137      {
     1138        "_name": "8",
     1139        "_type": "number",
     1140        "_value": "0"
     1141      },
     1142      {
     1143        "_name": "9",
     1144        "_type": "number",
     1145        "_value": "0"
     1146      }
     1147    ],
     1148    "_entries": null
     1149  }
     1150}
     1151
     1152-----------------------------------------------------
    12021153EXPRESSION: ({})
    12031154{
    … …  
    12621213        "_name": "c",
    12631214        "_type": "number",
    1264         "_value": "0"
     1215        "_value": "-0"
    12651216      }
    12661217    ],

  • trunk/LayoutTests/inspector/model/remote-object.html

    r185237 r185741  
    8888        {expression: "new Int32Array(new ArrayBuffer(16))"},
    8989        {expression: "var intArray = new Int32Array(new ArrayBuffer(16)); for (var i = 0; i < intArray.length; ++i) intArray[i] = i; intArray"},
     90        {expression: "var buffer = new ArrayBuffer(10000000); var int8View = new Int8Array(buffer); int8View"}, // 10000000 elements.
    9091
    9192        // Object

  • trunk/Source/JavaScriptCore/ChangeLog

    r185728 r185741  
     12015-06-19  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
     4        https://bugs.webkit.org/show_bug.cgi?id=146141
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        * inspector/InjectedScriptSource.js:
     9        (InjectedScript.prototype._propertyDescriptors):
     10        Avoid calling getOwnPropertyNames/Symbols on very large lists. Instead
     11        just generate property descriptors for the first 100 indexes. Note
     12        this would behave poorly for sparse arrays with a length > 100, but
     13        general support for lists with more than 100 elements is poor. See:
     14        <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
     15
    1162015-06-18  Yusuke Suzuki  <utatane.tea@gmail.com>
    217

  • trunk/Source/JavaScriptCore/inspector/InjectedScriptSource.js

    r185540 r185741  
    699699        }
    700700
    701         // Iterate prototype chain.
     701        function arrayIndexPropertyNames(o, length)
     702        {
     703            var array = new Array(length);
     704            for (var i = 0; i < length; ++i) {
     705                if (i in o)
     706                    array.push("" + i);
     707            }
     708            return array;
     709        }
     710
     711        // FIXME: <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
     712        // For array types with a large length we attempt to skip getOwnPropertyNames and instead just sublist of indexes.
     713        var isArrayTypeWithLargeLength = false;
     714        try {
     715            isArrayTypeWithLargeLength = injectedScript._subtype(object) === "array" && isFinite(object.length) && object.length > 100;
     716        } catch(e) {}
     717
    702718        for (var o = object; this._isDefined(o); o = o.__proto__) {
    703719            var isOwnProperty = o === object;
    704             processProperties(o, Object.getOwnPropertyNames(o), isOwnProperty);
    705             if (Object.getOwnPropertySymbols)
    706                 processProperties(o, Object.getOwnPropertySymbols(o), isOwnProperty);
     720
     721            if (isArrayTypeWithLargeLength && isOwnProperty)
     722                processProperties(o, arrayIndexPropertyNames(o, 100), isOwnProperty);
     723            else {
     724                processProperties(o, Object.getOwnPropertyNames(o), isOwnProperty);
     725                if (Object.getOwnPropertySymbols)
     726                    processProperties(o, Object.getOwnPropertySymbols(o), isOwnProperty);
     727            }
     728
    707729            if (collectionMode === InjectedScript.CollectionMode.OwnProperties)
    708730                break;

  • trunk/Source/WebInspectorUI/ChangeLog

    r185734 r185741  
     12015-06-19  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Avoid getOwnPropertyNames/Symbols on very large lists
     4        https://bugs.webkit.org/show_bug.cgi?id=146141
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        * UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:
     9        (WebInspector.JavaScriptRuntimeCompletionProvider.prototype.completionControllerCompletionsNeeded):
     10        When building completions for a large array, instead of building a list of all the indices
     11        just send a single number back to the frontend and let it generate a list for completion.
     12        We should do even better and avoid building a lookup table here for completion of an index.
     13
    1142015-06-18  Matt Baker  <mattbaker@apple.com>
    215

  • trunk/Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js

    r182579 r185741  
    126126            }
    127127
     128            function getArrayCompletions(primitiveType)
     129            {
     130                var array = this;
     131                var arrayLength;
     132
     133                var resultSet = {};
     134                for (var o = array; o; o = o.__proto__) {
     135                    try {
     136                        if (o === array && o.length) {
     137                            // If the array type has a length, don't include a list of all the indexes.
     138                            // Include it at the end and the frontend can build the list.
     139                            arrayLength = o.length;
     140                        } else {
     141                            var names = Object.getOwnPropertyNames(o);
     142                            for (var i = 0; i < names.length; ++i)
     143                                resultSet[names[i]] = true;
     144                        }
     145                    } catch (e) {
     146                        // Ignore
     147                    }
     148                }
     149
     150                if (arrayLength)
     151                    resultSet["length"] = arrayLength;
     152
     153                return resultSet;
     154            }
     155
    128156            function getCompletions(primitiveType)
    129157            {
    … …  
    152180            }
    153181
    154             if (result.type === "object" || result.type === "function")
     182            if (result.subtype === "array")
     183                result.callFunctionJSON(getArrayCompletions, undefined, receivedArrayPropertyNames.bind(this));
     184            else if (result.type === "object" || result.type === "function")
    155185                result.callFunctionJSON(getCompletions, undefined, receivedPropertyNames.bind(this));
    156186            else if (result.type === "string" || result.type === "number" || result.type === "boolean")
    … …  
    163193        {
    164194            receivedPropertyNames.call(this, result && !wasThrown ? result.value : null);
     195        }
     196
     197        function receivedArrayPropertyNames(propertyNames)
     198        {
     199            // FIXME: <https://webkit.org/b/143589> Web Inspector: Better handling for large collections in Object Trees
     200            // If there was an array like object, we generate autocompletion up to 1000 indexes, but this should
     201            // handle a list with arbitrary length.
     202            if (propertyNames && typeof propertyNames.length === "number") {
     203                var max = Math.min(propertyNames.length, 1000);
     204                for (var i = 0; i < max; ++i)
     205                    propertyNames[i] = true;
     206            }
     207
     208            receivedPropertyNames.call(this, propertyNames);
    165209        }
    166210
Note: See TracChangeset for help on using the changeset viewer.