Footprint increase - acmeair after load last 2 weeks

[mhdawson]

https://benchmarking.nodejs.org/charts/acmeair_postfootprint.png

Looks to have increased from around 90 to ~126.

@targos, does this line up with an update to v8 ?

[mhdawson]

This there is also an increase footprint after for node-dc-eis.

[targos]

Do you have an exact date for the first point that is higher?

[targos]

Ping @mhdawson. Is the raw data public?

[mhdawson]

The raw data is not published right now, but I can post it here.

[mhdawson]

Discussed in last Benchmarking meeting @uttampawar took action to try to narrow it down to a date/commit

[mhdawson]

Turns out I need to do a sql query to get the raw data so will take me a bit longer.

[mhdawson]

Raw data is in:

acmeair_postfootprint.txt

Strangely I can't find the increased numbers in the data yet. Is should be in the series were we see

  data6.push({
    x: new Date(1518767054000),
    y: 89616
  });
  data6.push({
    x: new Date(1518853564000),
    y: 92152
  });
  data6.push({
    x: new Date(1518939842000),
    y: 89720
  });
  data6.push({
    x: new Date(1519025877000),
    y: 89648
  });
  data6.push({
    x: new Date(1519112241000),
    y: 93996
  });
  data6.push({
    x: new Date(1519199005000),
    y: 91884
  });
  data6.push({
    x: new Date(1519285666000),
    y: 90684
  });
  data6.push({
    x: new Date(1519372853000),
    y: 90160
  });
  data6.push({
    x: new Date(1519457996000),
    y: 92700
  });

etc.

[targos]

data6 is v8.x. With data1 (master), we have this:

First problematic point is on 2018-02-02. There was no V8 update this time (it happened 5 days before). I'm not surprised because the canary curve followed master. We should have seen the regression in canary before.

Many commits landed on 2018-02-01, so it's difficult to find candidates for the regression.
acmeair is a benchmark for HTTP, right?
The most obvious ones would be:

• nodejs/node@5898dc3
• nodejs/node@7c4b09b
• nodejs/node@4404c76

[addaleax]

I think nodejs/node@5898dc3 is pretty safe, it shouldn’t change memory layout in the big picture, and only has an effect if there’s handles being sent over an IPC channel.

I wouldn’t consider it impossible that nodejs/node@7c4b09b has a memory impact. I would be surprised, but from a quick look at the commits landed that day it seems like it would be reasonable candidate.

I can’t say anything about nodejs/node@4404c76.

I wonder if nodejs/node@67a4ce1 would be a candidate too. It certainly has performance implications, although maybe not so much for memory usage?

[uttampawar]

@mhdawson @addaleax Today, node-dc-eis shows +7% and acme-air shows +11% memory footprint increase after-load compare to last week where it was +30% higher as measured on the master branch.

[mhdawson]

An additional +7% and 11% or down to only being that versus the previous +30%

[uttampawar]

I'm still narrowing it down from Commit list in February. Currently we do see stable size for node-dc-eis (much closer to the LTS 8) but AcmeAir show relatively high footprint. I'll update the result as soon as I find the culprit commit.

[uttampawar]

For Node-DC-EIS I see following footprint increase,

commit id: 4404c76
metric pre footprint 49104
metric post footprint 123868
metric footprint increase 74764

commit id: 5898dc3
metric pre footprint 48072
metric post footprint 143656
metric footprint increase 95584

commit id: 7c4b09b
metric pre footprint 48824
metric post footprint 147052
metric footprint increase 98228

Release: (v8.x)
metric pre footprint 48156
metric post footprint 130104
metric footprint increase 81948

[uttampawar]

It looks like, commit-id 7c4b09b where we see significant footprint increase. See following table for 15 commits.

Commit-id (pre, post,increase)

5898dc3 src: simplify handles for libuv streams (48072, 143656, 95584)
7c4b09b src: refactor stream callbacks and ownership (48824, 147052, 98228)
1b6cb94 benchmark: refactor (49108, 124508, 75400)
c713f07 benchmark: (timers) refactor
1178670 benchmark: (http(2)) refactor
2072f34 benchmark: (es) refactor
1a8ed22 benchmark: (url) refactor
39ae0b9 benchmark: (crypto) refactor
794e489 benchmark: (buffer) refactor
cd7b2c4 benchmark: (assert) refactor (49240, 127284, 78044)
f951c9a benchmark: fix variables not being set
d4015b8 benchmark: fix platform in basename-win32
ff21fb1 crypto: use non-deprecated v8::Object::Set (48840, 123432, 74592)
4404c76 http: process headers after setting up agent (49104, 123868, 74764)
7d4b772 test: fix flaky test-http-dns-error (49372, 124940, 75568)

[addaleax]

I think this was fixed by nodejs/node@3217e8e

[mhdawson]

Chart does not seem to be updated, but looking at the output from the 10.x run it did look better:

metric pre footprint 79908
metric post footprint 89576

which is close to what we were seeing for 8.x

[mhdawson]

Latest chart also shows that its back to expected values :) Closing.