Bug 175100 - [Linux] JSTests/wasm/stress/oom.js should not run on Linux
Summary: [Linux] JSTests/wasm/stress/oom.js should not run on Linux
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-02 16:58 PDT by Carlos Alberto Lopez Perez
Modified: 2017-08-03 10:41 PDT (History)
12 users (show)

See Also:


Attachments
Patch (1.03 KB, patch)
2017-08-02 17:06 PDT, Carlos Alberto Lopez Perez
mark.lam: review+
mark.lam: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2017-08-02 16:58:23 PDT
The JSC test JSTests/wasm/stress/oom.js seems to be specifically designed to try to use all the available memory until an out of memory exception happens.

Trying to do something like that on Linux is not fine, as Linux handles very badly out of memory situations. In the best case the system will freeze during several seconds and in the worst case it may be several minutes until it starts responding back.

We should try to avoid running tests like this on Linux.
Comment 1 Carlos Alberto Lopez Perez 2017-08-02 17:06:22 PDT
Created attachment 317035 [details]
Patch
Comment 2 Mark Lam 2017-08-02 17:13:34 PDT
Comment on attachment 317035 [details]
Patch

r=me.  Please add your rationale for why this change is needed in the ChangeLog.  Thanks.
Comment 3 JF Bastien 2017-08-02 17:34:20 PDT
I don’t think this is the right fix. Can you instead run under ulimit ?
Comment 4 Filip Pizlo 2017-08-02 17:35:57 PDT
Comment on attachment 317035 [details]
Patch

r=me too.  I know that you elaborated in the bugzilla comment, but it would be even nicer if the ChangeLog also contained the text about why this test doesn't make sense on Linux.
Comment 5 Filip Pizlo 2017-08-02 17:37:19 PDT
(In reply to JF Bastien from comment #3)
> I don’t think this is the right fix. Can you instead run under ulimit ?

I think it's more pragmatic to just do this fix.  We're always open to ports skipping tests without much fuss.  In particular, I would have even been OK with Carlos landing this change without review, since test gardening technically does not need review.
Comment 6 Carlos Alberto Lopez Perez 2017-08-02 18:36:08 PDT
(In reply to JF Bastien from comment #3)
> I don’t think this is the right fix. Can you instead run under ulimit ?

ulimit is pretty broken on Linux, as limiting by rss (ulimit -m) doesn't work at all on Linux (since kernels 2.4.x) and limiting by vsize (ulimit -v ) requires some hard-to-get-right ad-hoc knowledge about how much virtual size is acceptable for a process to allocate.

For example, this test is allocating ~15GB of RSS and ~57GB of VSZ before giving the out of memory error on a Linux machine with 16GB of RAM. So, how should I know that I should set an ulimit on 57GB other than by try and error? Playing with limiting the vsize seems a bit fragile to me.

The only thing that I know works ok on Linux for limiting memory usage are cgroups. But cgroups are meant to limit the memory usage of a group of process (its usually used with containers). They are not really meant to limit the memory usage of a single process like we would like to do in this case.

There is also the issue that this problem will not only trigger on the bots, but also on the developers machines. Any WebKit Linux developer trying to run this test will likely run into desktop freezes.


I'm seizing this to comment also about a related issue at https://bugs.webkit.org/show_bug.cgi?id=171630#c11 where I observed that WebKit/JSC is pretty unique in the sense that we allow a script to use all possible RAM of the system. Other JS engines seem to limit this value to some sane amount (like 4 GBs). I see how on MacOS allow this is not a problem as the Mac kernel seems to handle fine this situations, but on Linux we run into problems when this happens.
Comment 7 Carlos Alberto Lopez Perez 2017-08-02 18:42:23 PDT
Committed r220173: <http://trac.webkit.org/changeset/220173>
Comment 8 Radar WebKit Bug Importer 2017-08-02 18:43:30 PDT
<rdar://problem/33691382>
Comment 9 JF Bastien 2017-08-02 19:08:25 PDT
(In reply to Carlos Alberto Lopez Perez from comment #6)
> (In reply to JF Bastien from comment #3)
> > I don’t think this is the right fix. Can you instead run under ulimit ?
> 
> ulimit is pretty broken on Linux, as limiting by rss (ulimit -m) doesn't
> work at all on Linux (since kernels 2.4.x) and limiting by vsize (ulimit -v
> ) requires some hard-to-get-right ad-hoc knowledge about how much virtual
> size is acceptable for a process to allocate.
> 
> For example, this test is allocating ~15GB of RSS and ~57GB of VSZ before
> giving the out of memory error on a Linux machine with 16GB of RAM. So, how
> should I know that I should set an ulimit on 57GB other than by try and
> error? Playing with limiting the vsize seems a bit fragile to me.

The test just tries to run out. Doesn’t matter at how much memory. 

> The only thing that I know works ok on Linux for limiting memory usage are
> cgroups. But cgroups are meant to limit the memory usage of a group of
> process (its usually used with containers). They are not really meant to
> limit the memory usage of a single process like we would like to do in this
> case.
> 
> There is also the issue that this problem will not only trigger on the bots,
> but also on the developers machines. Any WebKit Linux developer trying to
> run this test will likely run into desktop freezes.
> 
> 
> I'm seizing this to comment also about a related issue at
> https://bugs.webkit.org/show_bug.cgi?id=171630#c11 where I observed that
> WebKit/JSC is pretty unique in the sense that we allow a script to use all
> possible RAM of the system. Other JS engines seem to limit this value to
> some sane amount (like 4 GBs). I see how on MacOS allow this is not a
> problem as the Mac kernel seems to handle fine this situations, but on Linux
> we run into problems when this happens.