Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(229)

Issue 254273002: Remove BootUpMemoryUse test. (Closed)

Created:
6 years, 7 months ago by Hannes Payer (out of office)
Modified:
6 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -24 lines) Patch
M test/cctest/test-mark-compact.cc View 1 chunk +0 lines, -24 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Benedikt Meurer
Yay! LGTM
6 years, 7 months ago (2014-04-30 08:18:54 UTC) #1
Hannes Payer (out of office)
Committed patchset #1 manually as r21073 (presubmit successful).
6 years, 7 months ago (2014-04-30 08:19:44 UTC) #2
Jakob Kummerow
DBC: I'm not convinced that removing this test is a good idea. Yes, bumping the ...
6 years, 7 months ago (2014-05-05 08:28:41 UTC) #3
Dmitry Lomov (no reviews)
On 2014/05/05 08:28:41, Jakob wrote: > DBC: I'm not convinced that removing this test is ...
6 years, 7 months ago (2014-05-05 08:52:24 UTC) #4
Hannes Payer (out of office)
6 years, 7 months ago (2014-05-05 17:10:30 UTC) #5
Message was sent while issue was closed.
On 2014/05/05 08:52:24, Dmitry Lomov (chromium) wrote:
> On 2014/05/05 08:28:41, Jakob wrote:
> > DBC: I'm not convinced that removing this test is a good idea.
> > Yes, bumping the limit every so often is annoying and kind of defeats the
> > purpose of the test. On the other hand, we put in a lot of work to get
boot-up
> > memory use low (IIRC the effort at the time brought it from ~20 to ~2 MB),
and
> > as we know, every metric we don't track is bound to regress over time. So I
> > think in spite of its drawbacks, this test did serve a valid purpose. I vote
> for
> > putting it back in.
> 
> +1 to keeping us honest about boot-up memory

Feel free to revert this change and bump up the limit to something acceptable...

Powered by Google App Engine
This is Rietveld 408576698