Unable to reproduce the issue on win8.1 chrome canary version #53.0.2760.0 - Page works fine on scrolling up and down

Could you please replicate the same on a new profile where there are no apps/extensions and update the thread. You can create a new profile from chrome://settings

I just tried again in incognito mode in canary. I scrolled for a few sceonds and the page hung. I did also try in a new profile and still the same issue. Win7 Version 53.0.2761.2 canary (64-bit)

Oh -- maybe this requires a high-dpi display??

Thank you for providing more feedback. Adding requester "tkonchada@chromium.org" for another review and adding "Needs-Review" label for tracking.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I was able reproduce on Linux 53.0.2759.0 (64-bit)

The issue is that we seem to have a leak somewhere, so the V8 GC cannot reduce the memory footprint. After a couple seconds we try to GC a 1.4GB heap, which takes some time.

Taking a look now.

Yuki, can you please have a look. The issue seems pretty severe as the leak results in OOM. It also happens on ToT.

The change is crrev.com/7aff51476ff48588879709275a3c363c6114e085

Also: crrev.com/9ac1750adf85027985c7af3468e0b972c2086235 seems to fix it. But since it got reverted we are in a broken state again.

Bisect points to
  https://chromium.googlesource.com/chromium/src/+log/76ea7af93b5532777ee787fc4f3039ddfa6125d3..7aff51476ff48588879709275a3c363c6114e085

$ tools/bisect-builds.py -g 386251 -b 390892 -a linux64 --use-local-cache -- --no-first-run --no-sandbox
Downloading list of known revisions...
Loaded revisions 41523-398850 from /usr/local/google/home/mlippautz/ssd/chromium-tot/src/tools/.bisect-builds-cache.json
Downloading revision 388495...
Received 84611482 of 84611482 bytes, 100.00%
Bisecting range [386257 (good), 390892 (bad)].
Trying revision 388495...
Revision 388495 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 387314...
Bisecting range [386257 (good), 388495 (bad)].
Trying revision 387314...
Revision 387314 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 387882...
Bisecting range [387314 (good), 388495 (bad)].
Trying revision 387882...
Revision 387882 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 388189...
Bisecting range [387882 (good), 388495 (bad)].
Trying revision 388189...
Revision 388189 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 388083...
Bisecting range [387882 (good), 388189 (bad)].
Trying revision 388083...
Revision 388083 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 388144...
Bisecting range [388083 (good), 388189 (bad)].
Trying revision 388144...
Revision 388144 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 388169...
Bisecting range [388144 (good), 388189 (bad)].
Trying revision 388169...
Revision 388169 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: g
Downloading revision 388179...
Bisecting range [388169 (good), 388189 (bad)].
Trying revision 388179...
Revision 388179 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 388174...
Bisecting range [388169 (good), 388179 (bad)].
Trying revision 388174...
Revision 388174 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
Downloading revision 388170...
Bisecting range [388169 (good), 388174 (bad)].
Trying revision 388170...
Revision 388170 is [(g)ood/(b)ad/(r)etry/(u)nknown/(s)tdout/(q)uit]: b
You are probably looking for a change made after 388169 (known good), but no later than 388170 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/76ea7af93b5532777ee787fc4f3039ddfa6125d3..7aff51476ff48588879709275a3c363c6114e085

What the website is doing is (probably) the followings:
a) makes |window| have a circular structure, and
b) JSON.stringify(window)

Somehow the ToT seems failing to detect the circular structure and, as a result, it doesn't throw an exception.  I guess stringifying the circular structure is causing OOM.

#12 was (partially) wrong.  As I investigated the issue, the cause seems
a) the website is creating an array with length = 142228180, and
b) trying to serialize it.
It seems depending on Chrome version whether the website creates and serialize such an array or not.  With Chrome M51, the website doesn't create or serialize such an array.  However, with Chrome M53, the website does it.

Plus, somehow V8 became to take more time or more memory until giving up the serialization.
    x = new Array
    x.length = 142228180
    JSON.serialize(x)
When I ran the above code with M51 on my local machine, it consumed 1.8 GB memory and gave up the serialization throwing "Uncaught RangeError: Invalid string length".
With ToT, it consumed 3.0 GB and gave up with the same exception.

If you wait for an enough long time that V8 gives up the serialization, everything works fine.  The page does not crash.  Scroll has nothing to do with this issue.  Even if you did nothing, the website tries to serialize a big array.

My conclusion is that binding's change caused the website to create and serialize an array with length = 142,228,180, and it made the page super heavy.  Thus, this is an issue of the site.

I'll close this issue as WontFix if no one raised concerns.

s/JSON.serialize/JSON.stringify/g

Maybe we should fail faster for a case like this?

I confirmed that |length = 142228180| is passed to JsonStringifier::SerializeArrayLikeSlow() defined in v8/src/json-stringifier.cc .  JSON.stringify() is implemented in V8.

Probably we can fail faster around there (if we really want).

V8 team (jochen@, verwaest@), what do you guys think?

+Yang

I don't think there is a foolproof solution to this. If someone wants to serialize a big array, it's going to take a long time, and we only realize later that we will run out of memory.

What we could do for this particular case is to do an estimate:
- we need to allocate at least two characters per array element
- the maximum string length is String::kMaxLength = (1 << 28) - 16
- 142228180 > 134217720, so we can throw with a RangeError earlier.

But I'm not sure this is something we want to do. If the array is only slightly shorter, we run into the same issue.

There is something else I can fix though.

Can someone verify whether https://codereview.chromium.org/2069563002 leads to JSON.stringify failing sooner?

You can confirm it with
    x = new Array
    x.length = 142228180
    JSON.stringify(x)

I confirmed that it fails a little bit earlier than before.  However, it still takes some time.  For the website of the issue, it took about 20 sec by giving up on my local machine.  But it's better than before.

In terms of memory, it gave up when it consumed 1.4 GB.  Better than 3.0 GB (M53) or 1.8 GB (M51).

I uploaded a new patchset that also addresses the issue in this case. But as I said previously, it's not foolproof.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/19067e5fbc3da97f87a4b430a1b589529723cdbd

commit 19067e5fbc3da97f87a4b430a1b589529723cdbd
Author: yangguo <yangguo@chromium.org>
Date: Tue Jun 14 11:34:17 2016

[json] detect overflow sooner when serializing large sparse array.

R=mlippautz@chromium.org, yukishiino@chromium.org
BUG= chromium:617649 

Review-Url: https://codereview.chromium.org/2069563002
Cr-Commit-Position: refs/heads/master@{#36961}

[modify] https://crrev.com/19067e5fbc3da97f87a4b430a1b589529723cdbd/src/json-stringifier.cc
[modify] https://crrev.com/19067e5fbc3da97f87a4b430a1b589529723cdbd/src/string-builder.h

Thanks Yang for the quick fix.  I confirmed with ToT that it works fine with the website of the issue (for this specific case).  I close this issue as Fixed.