Bug 170412

Summary: Fix incorrect capacity delta calculation reported in SparseArrayValueMap::add().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, fpizlo, ggaren, jfbastien, keith_miller, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
fpizlo: review+, buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-elcapitan
none
Patch for landing: w/ 1 fix.
none
patch for landing w/ test case. none

Mark Lam
Reported 2017-04-03 13:15:51 PDT
Also fix Heap's extra memory size accounting to handle overflows. <rdar://problem/29697336>
Attachments
proposed patch. (7.25 KB, patch)
2017-04-03 13:51 PDT, Mark Lam
fpizlo: review+
buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-elcapitan (1.22 MB, application/zip)
2017-04-03 14:23 PDT, Build Bot
no flags
Patch for landing: w/ 1 fix. (7.25 KB, patch)
2017-04-03 14:52 PDT, Mark Lam
no flags
patch for landing w/ test case. (8.19 KB, patch)
2017-04-03 17:23 PDT, Mark Lam
no flags
Mark Lam
Comment 1 2017-04-03 13:51:37 PDT
Created attachment 306104 [details] proposed patch.
Build Bot
Comment 2 2017-04-03 14:23:20 PDT
Comment on attachment 306104 [details] proposed patch. Attachment 306104 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3467174 Number of test failures exceeded the failure limit.
Build Bot
Comment 3 2017-04-03 14:23:21 PDT
Created attachment 306115 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 4 2017-04-03 14:26:15 PDT
Comment on attachment 306104 [details] proposed patch. Attachment 306104 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/3467269 New failing tests: stress/recursive-try-catch.js.ftl-no-cjit-small-pool stress/recursive-try-catch.js.dfg-eager-no-cjit-validate stress/recursive-try-catch.js.no-ftl stress/recursive-try-catch.js.dfg-maximal-flush-validate-no-cjit stress/recursive-try-catch.js.no-cjit-validate-phases stress/recursive-try-catch.js.ftl-no-cjit-no-inline-validate stress/recursive-try-catch.js.dfg-eager stress/recursive-try-catch.js.ftl-no-cjit-no-put-stack-validate stress/recursive-try-catch.js.no-cjit-collect-continuously stress/recursive-try-catch.js.ftl-eager-no-cjit stress/recursive-try-catch.js.default stress/recursive-try-catch.js.no-llint stress/new-largeish-contiguous-array-with-size.js.ram-size-10000000 stress/recursive-try-catch.js.ftl-eager
Mark Lam
Comment 5 2017-04-03 14:52:54 PDT
Created attachment 306120 [details] Patch for landing: w/ 1 fix. In Heap::extraMemorySize(), I meant to take the min of (total, std::numeric_limits<size_t>::max() - m_objectSpace.capacity()), not the max. This is the reason for the JSC test failure and possibly the layout test failures too. Let's try this on the EWS bots.
Geoffrey Garen
Comment 6 2017-04-03 15:06:25 PDT
Comment on attachment 306120 [details] Patch for landing: w/ 1 fix. This is the kind of change where the test is usually more valuable than the code change. Can you think of a way to test this?
Mark Lam
Comment 7 2017-04-03 17:15:18 PDT
(In reply to Geoffrey Garen from comment #6) > Comment on attachment 306120 [details] > Patch for landing: w/ 1 fix. > > This is the kind of change where the test is usually more valuable than the > code change. Can you think of a way to test this? I found a way to make a test. Will add it to the patch.
Mark Lam
Comment 8 2017-04-03 17:23:57 PDT
Created attachment 306145 [details] patch for landing w/ test case.
Mark Lam
Comment 9 2017-04-03 17:42:53 PDT
Thanks for the review. Landed in r214857: <http://trac.webkit.org/r214857>.
Note You need to log in before you can comment on or make changes to this bug.