1084324 - Chained Promises cause high memory usage

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: promise_test.html

Description:
It appears that chained Promises do not release unreachable resources even each of them is then()'ed; as long as the last promise in the chain is still referenceable.

STR: 
1. Please download the attachment, a test html.
2. Open that attachment with Nightly.
3. Open about:memory, do a measurement.
4. Click "run" in the test html. As it creates 1 million Promises it can take some time to finish. When it finishes you see a "done" string in the html view.
5. In about:memory, do a measurement. Observe that there is high usage in the html page's js compartment.
6. In about:memory, click GC, and CC.
7. Do a measurement again.

Actual result: Memory measurement indicates the test html still occupies huge amount of memory (in my case ~100MB).

Expected result: Memory measurement indicates minimal memory usage for the test html, probably not much larger than the measurement in step 3 (~a couple hundreds KB).

Test with today's Nightly on Ubuntu 14.04 x64, about:buildconfig indicates revision as https://hg.mozilla.org/mozilla-central/rev/a280a03c9f3c

I suppose that only the reference of last Promise is needed?

Following is the fragment of memory measurement about:memory in Step 7:
│  │  ├──123,240,576 B (14.42%) -- js-compartment(file:///home/purin/Desktop/promise_test.html)
│  │  │  │  ├──123,220,168 B (14.42%) -- classes
│  │  │  │  │  ├───72,307,160 B (08.46%) -- class(Function)
│  │  │  │  │  │   ├──72,284,800 B (08.46%) -- objects
│  │  │  │  │  │   │  ├──72,280,256 B (08.46%) ── gc-heap
│  │  │  │  │  │   │  └───────4,544 B (00.00%) ── malloc-heap/slots
│  │  │  │  │  │   └──────22,360 B (00.00%) -- shapes
│  │  │  │  │  │          ├──22,264 B (00.00%) -- gc-heap
│  │  │  │  │  │          │  ├──17,112 B (00.00%) ── tree
│  │  │  │  │  │          │  └───5,152 B (00.00%) ── base
│  │  │  │  │  │          └──────96 B (00.00%) ── malloc-heap/tree-kids
│  │  │  │  │  ├───50,837,904 B (05.95%) -- class(Promise)
│  │  │  │  │  │   ├──50,837,808 B (05.95%) ── objects/gc-heap
│  │  │  │  │  │   └──────────96 B (00.00%) -- shapes/gc-heap
│  │  │  │  │  │              ├──56 B (00.00%) ── base
│  │  │  │  │  │              └──40 B (00.00%) ── tree

And this is after I click Minimize memory usage (after clicking GC and CC) in Step 7:
│  │  │   ├──90,114,384 B (10.69%) -- js-compartment(file:///home/purin/Desktop/promise_test.html)
│  │  │   │  ├──90,095,672 B (10.68%) -- classes
│  │  │   │  │  ├──47,576,952 B (05.64%) -- class(Function)
│  │  │   │  │  │  ├──47,554,592 B (05.64%) -- objects
│  │  │   │  │  │  │  ├──47,550,048 B (05.64%) ── gc-heap
│  │  │   │  │  │  │  └───────4,544 B (00.00%) ── malloc-heap/slots
│  │  │   │  │  │  └──────22,360 B (00.00%) -- shapes
│  │  │   │  │  │         ├──22,264 B (00.00%) -- gc-heap
│  │  │   │  │  │         │  ├──17,112 B (00.00%) ── tree
│  │  │   │  │  │         │  └───5,152 B (00.00%) ── base
│  │  │   │  │  │         └──────96 B (00.00%) ── malloc-heap/tree-kids
│  │  │   │  │  ├──42,443,616 B (05.03%) -- class(Promise)
│  │  │   │  │  │  ├──42,443,520 B (05.03%) ── objects/gc-heap
│  │  │   │  │  │  └──────────96 B (00.00%) -- shapes/gc-heap
│  │  │   │  │  │             ├──56 B (00.00%) ── base
│  │  │   │  │  │             └──40 B (00.00%) ── tree

Comment 1

[Boris Zbarsky [:bzbarsky]]

This testcase actually creates 2 million promises, not 1 million.  One promise per call to then() and one promise per invocation of the "then" callback.

Try stepping through it: once the loop terminates, we start calling the "then" callbacks for that very first promise.  At this point there are 1e6 promises alive.  We call the first callback, it returns a promise.  That means we now have to wait for that promise to settle before our original promise is settled, so we do that (this requires a trip through the event loop).  When it settles, we call the second then function, this returns a promise, we have to wait for it to settle, etc.

The upshot is that we have 1 million promises in memory until we have called the last "then" callback the promise chain, which takes 1 million trips through the event loop.  Then the promise chain starts actually shrinking, and after 1 million more trips there's only one promise live.

You can see this clearly if you modify the testcase to do the "done" thing only when global_p is actually resolved:

  global_p.then(function() {
    document.getElementById('msg').appendChild(document.createTextNode('done'));
  });

and note how long it takes to get there.  Once you do get there, about:memory shows basically no Promise usage, especially after minimizing.

The patch for bug 1013625 will likely help here somewhat, by converting the async bits into something more sync, so it will be quite clear that we're not done with the promise chain yet.  Of course that will be more janky than what we have now...

So as far a I can tell this is invalid; this testcase very purposefully makes sure that a lot of CPU time needs to elapse in a state with 1 million live promises.

Comment 2

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: promise_test_with_wait.html

Thanks for your information, :bz!

So from what I understand, you meant that after my original run() ends, Gecko is still spending CPU time dealing with those hundreds of thousands of Promises in the event queue; thus it did not accurately measure memory usage if I took a memory measurement *immediately* after the run() ended (as those Promises hadn't been all processed).

I can confirm what you said: if I wait for a bit longer (with my laptop it's about 100 seconds) after |run()| before taking the memory measurement, (and I still hold the reference to the last promise, and am not then()'ing it), I can no longer observe tremendous memory usage after GC & CC calls.

(My modified attachment in this comment tells you how many seconds have been waited for, it's not technically different from my original attachment in terms of test purposes :p)

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

(Tim: this is just a FYI)

Following comment 1 and comment 2, my original concern regarding how bug 1083638 could be affected by this bug is no longer valid and this bug by itself should be invalid too.

For bug 1083638, the promise processing is probably light enough as we only attach a couple or two Promises each time we queue the tasks.

Comment 4

[John Lu [:mnjul][Please NI or I'll miss]]

And I actually forgot to NI tim at comment 3. (facepalm)

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Got it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment 2's understanding is exactly correct.