Undocumented breaking change in a minor

[rudolf]

Version

16.7.0

Platform

No response

Subsystem

perf_hooks

What steps will reproduce the bug?

From @pmuellr elastic/kibana#116636 (comment)

const { performance } = require('perf_hooks')

const MarksPerChunk = 1000 * 1000 // ONE MILLION MARKS
let marks = 0
setInterval(printMarks, 1000)
setImmediate(add_marks_endlessly)

// make it async-y, to give printMarks a chance to run
function add_marks_endlessly() {
  for (let i=0; i<MarksPerChunk; i++) {
    performance.mark('another')
    marks++
  }
  setImmediate(add_marks_endlessly)
}

function printMarks() {
  console.log(`${new Date().toISOString()} ${marks / MarksPerChunk}M marks`)
}

$ ~/.nvm/versions/node/v16.7.0/bin/node --max_old_space_size=1024 perf-mark.js
(node:59477) MaxPerformanceEntryBufferExceededWarning: Possible perf_hooks memory leak detected. 1000001 mark entries added to the global performance entry buffer. Use performance.clearMarks to clear the buffer.
(Use `node --trace-warnings ...` to show where the warning was created)
2021-11-08T13:46:20.422Z 3M marks
2021-11-08T13:46:21.718Z 6M marks
2021-11-08T13:46:23.245Z 9M marks
2021-11-08T13:46:24.748Z 11M marks
2021-11-08T13:46:28.010Z 12M marks

<--- Last few GCs --->

[59477:0x7fc998008000]    10512 ms: Scavenge 1020.4 (1042.1) -> 1019.9 (1044.3) MB, 1.6 / 0.0 ms  (average mu = 0.175, current mu = 0.150) allocation failure 


<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

$ ~/.nvm/versions/node/v14.17.6/bin/node --max_old_space_size=1024 perf-mark.js
2021-11-04T20:41:58.826Z 1M marks
2021-11-04T20:42:00.043Z 2M marks
2021-11-04T20:42:01.231Z 3M marks
2021-11-04T20:42:02.412Z 4M marks
2021-11-04T20:42:03.614Z 5M marks
2021-11-04T20:42:04.846Z 6M marks
2021-11-04T20:42:06.047Z 7M marks
2021-11-04T20:42:07.254Z 8M marks
2021-11-04T20:42:08.499Z 9M marks
...doesn't OOM ...

$ ~/.nvm/versions/node/v16.6.0/bin/node --max_old_space_size=1024 perf-mark.js
2021-11-04T20:41:58.826Z 1M marks
2021-11-04T20:42:00.043Z 2M marks
2021-11-04T20:42:01.231Z 3M marks
2021-11-04T20:42:02.412Z 4M marks
2021-11-04T20:42:03.614Z 5M marks
2021-11-04T20:42:04.846Z 6M marks
2021-11-04T20:42:06.047Z 7M marks
2021-11-04T20:42:07.254Z 8M marks
2021-11-04T20:42:08.499Z 9M marks
...doesn't OOM ...

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

1. A minor version upgrade of Nodejs should not lead to a memory leak.
2. When the behaviour of an API changes and consumers are expected to make changes to their code, this should be documented in the release notes.

What do you see instead?

The change was incorrectly classified as not introducing any breaking changes
#39297 (comment)

Additional information

Could we highlight this behaviour change in the documentation?

[Trott]

@legendecas

[legendecas]

Sorry for the incorrect categorization. I was thinking that the change didn't break the functionality of existing codes, but didn't categorize that introducing memory leaks on the non-spec-compliant behavior as breaking change. I agree that we should highlight on the document that for spec-compliance it is mandatory to call performance.clearMarks() to clear the performance timeline.

[legendecas]

Fixed with c071bd5