Bug 170412 - Fix incorrect capacity delta calculation reported in SparseArrayValueMap::add().
Summary: Fix incorrect capacity delta calculation reported in SparseArrayValueMap::add().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-03 13:15 PDT by Mark Lam
Modified: 2017-04-03 17:42 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch. (7.25 KB, patch)
2017-04-03 13:51 PDT, Mark Lam
fpizlo: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
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 Details
Patch for landing: w/ 1 fix. (7.25 KB, patch)
2017-04-03 14:52 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
patch for landing w/ test case. (8.19 KB, patch)
2017-04-03 17:23 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2017-04-03 13:15:51 PDT
Also fix Heap's extra memory size accounting to handle overflows.

<rdar://problem/29697336>
Comment 1 Mark Lam 2017-04-03 13:51:37 PDT
Created attachment 306104 [details]
proposed patch.
Comment 2 Build Bot 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.
Comment 3 Build Bot 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
Comment 4 Build Bot 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
Comment 5 Mark Lam 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.
Comment 6 Geoffrey Garen 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?
Comment 7 Mark Lam 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.
Comment 8 Mark Lam 2017-04-03 17:23:57 PDT
Created attachment 306145 [details]
patch for landing w/ test case.
Comment 9 Mark Lam 2017-04-03 17:42:53 PDT
Thanks for the review.  Landed in r214857: <http://trac.webkit.org/r214857>.