JavaScriptCore:
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jun 2007 04:58:04 +0000 (04:58 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jun 2007 04:58:04 +0000 (04:58 +0000)
        Reviewed by Mark Rowe.

        - fix http://bugs.webkit.org/show_bug.cgi?id=14132
          array sort with > 10000 elements sets elements > 10000 undefined

        Test: fast/js/sort-large-array.html

        * kjs/array_instance.h: Replaced pushUndefinedObjectsToEnd with
        compactForSorting, and removed ExecState parameters.

        * kjs/array_object.cpp:
        (ArrayInstance::sort): Changed to call compactForSorting.
        (ArrayInstance::compactForSorting): Do the get and delete of the
        properties directly on the property map instead of using public
        calls from JSObject. The public calls would just read the undefined
        values from the compacted sort results array!

LayoutTests:

        Reviewed by Mark Rowe.

        - test for http://bugs.webkit.org/show_bug.cgi?id=14132
          array sort with > 10000 elements sets elements > 10000 undefined

        * fast/js/resources/sort-large-array.js: Added.
        * fast/js/sort-large-array-expected.txt: Added.
        * fast/js/sort-large-array.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@23521 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/array_instance.h
JavaScriptCore/kjs/array_object.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/sort-large-array.js [new file with mode: 0644]
LayoutTests/fast/js/sort-large-array-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/sort-large-array.html [new file with mode: 0644]

index 40572ab..bc5c4c8 100644 (file)
@@ -1,3 +1,22 @@
+2007-06-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mark Rowe.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=14132
+          array sort with > 10000 elements sets elements > 10000 undefined
+
+        Test: fast/js/sort-large-array.html
+
+        * kjs/array_instance.h: Replaced pushUndefinedObjectsToEnd with
+        compactForSorting, and removed ExecState parameters.
+
+        * kjs/array_object.cpp:
+        (ArrayInstance::sort): Changed to call compactForSorting.
+        (ArrayInstance::compactForSorting): Do the get and delete of the
+        properties directly on the property map instead of using public
+        calls from JSObject. The public calls would just read the undefined
+        values from the compacted sort results array!
+
 2007-06-13  George Staikos  <staikos@kde.org>
 
         Reviewed by Lars.
index d7216a9..30358d1 100644 (file)
@@ -2,7 +2,7 @@
 /*
  *  This file is part of the KDE libraries
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003 Apple Computer, Inc.
+ *  Copyright (C) 2003, 2007 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -57,7 +57,7 @@ namespace KJS {
 
     void setLength(unsigned newLength, ExecState *exec);
     
-    unsigned pushUndefinedObjectsToEnd(ExecState *exec);
+    unsigned compactForSorting();
     
     void resizeStorage(unsigned);
 
index 624c290..4785c5e 100644 (file)
@@ -2,7 +2,7 @@
 /*
  *  This file is part of the KDE libraries
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003 Apple Computer, Inc.
+ *  Copyright (C) 2003, 2007 Apple Inc. All rights reserved.
  *  Copyright (C) 2003 Peter Kelly (pmk@post.com)
  *  Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
@@ -330,7 +330,7 @@ static int compareByStringForQSort(const void *a, const void *b)
 
 void ArrayInstance::sort(ExecState* exec)
 {
-    size_t lengthNotIncludingUndefined = pushUndefinedObjectsToEnd(exec);
+    size_t lengthNotIncludingUndefined = compactForSorting();
       
     ExecState* oldExec = execForCompareByStringForQSort;
     execForCompareByStringForQSort = exec;
@@ -397,7 +397,7 @@ static int compareWithCompareFunctionForQSort(const void *a, const void *b)
 
 void ArrayInstance::sort(ExecState* exec, JSObject* compareFunction)
 {
-    size_t lengthNotIncludingUndefined = pushUndefinedObjectsToEnd(exec);
+    size_t lengthNotIncludingUndefined = compactForSorting();
 
     CompareWithCompareFunctionArguments* oldArgs = compareWithCompareFunctionArguments;
     CompareWithCompareFunctionArguments args(exec, compareFunction);
@@ -422,7 +422,7 @@ void ArrayInstance::sort(ExecState* exec, JSObject* compareFunction)
     compareWithCompareFunctionArguments = oldArgs;
 }
 
-unsigned ArrayInstance::pushUndefinedObjectsToEnd(ExecState *exec)
+unsigned ArrayInstance::compactForSorting()
 {
     JSValue *undefined = jsUndefined();
 
@@ -447,8 +447,8 @@ unsigned ArrayInstance::pushUndefinedObjectsToEnd(ExecState *exec)
     PropertyNameArrayIterator end = sparseProperties.end();
     for (PropertyNameArrayIterator it = sparseProperties.begin(); it != end; ++it) {
         Identifier name = *it;
-        storage[o] = get(exec, name);
-        JSObject::deleteProperty(exec, name);
+        storage[o] = getDirect(name);
+        _prop.remove(name);
         o++;
     }
     
index 2762e04..95902e0 100644 (file)
@@ -1,3 +1,14 @@
+2007-06-13  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mark Rowe.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=14132
+          array sort with > 10000 elements sets elements > 10000 undefined
+
+        * fast/js/resources/sort-large-array.js: Added.
+        * fast/js/sort-large-array-expected.txt: Added.
+        * fast/js/sort-large-array.html: Added.
+
 2007-06-10  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Beth Dakin.
diff --git a/LayoutTests/fast/js/resources/sort-large-array.js b/LayoutTests/fast/js/resources/sort-large-array.js
new file mode 100644 (file)
index 0000000..ee25d0b
--- /dev/null
@@ -0,0 +1,14 @@
+description("This tests sorting an array with more than 10,000 values.");
+
+var test = [];
+for (var i = 0; i < 10010; i++)
+    test.push(10009 - i);
+test.sort(function(a,b) {return a - b;});
+
+shouldBe("test.length", "10010");
+shouldBe("test[9999]", "9999");
+shouldBe("test[10000]", "10000");
+shouldBe("test.slice(0, 20).join(', ')", "'0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19'");
+shouldBe("test.slice(9990, 10010).join(', ')", "'9990, 9991, 9992, 9993, 9994, 9995, 9996, 9997, 9998, 9999, 10000, 10001, 10002, 10003, 10004, 10005, 10006, 10007, 10008, 10009'");
+
+var successfullyParsed = true;
diff --git a/LayoutTests/fast/js/sort-large-array-expected.txt b/LayoutTests/fast/js/sort-large-array-expected.txt
new file mode 100644 (file)
index 0000000..2e201f2
--- /dev/null
@@ -0,0 +1,14 @@
+This tests sorting an array with more than 10,000 values.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS test.length is 10010
+PASS test[9999] is 9999
+PASS test[10000] is 10000
+PASS test.slice(0, 20).join(', ') is '0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19'
+PASS test.slice(9990, 10010).join(', ') is '9990, 9991, 9992, 9993, 9994, 9995, 9996, 9997, 9998, 9999, 10000, 10001, 10002, 10003, 10004, 10005, 10006, 10007, 10008, 10009'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/sort-large-array.html b/LayoutTests/fast/js/sort-large-array.html
new file mode 100644 (file)
index 0000000..d2029dd
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/sort-large-array.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>