-
Notifications
You must be signed in to change notification settings - Fork 30k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
vm: fix vm.measureMemory() and introduce execution option
nodejs/node-v8#147 broke the `vm.measureMemory()` API. It only created a `MeasureMemoryDelegate` and without actually calling `v8::Isolate::MeasureMemory()` so the returned promise will never resolve. This was not caught by the tests because the promise resolvers were not wrapped with `common.mustCall()`. This patch migrates the API properly and also introduce the newly added execution option to the API. It also removes support for specifying contexts to measure - instead we'll just return the measurements for all contexts in the detailed mode, which is what the `performance.measureMemory()` prototype in V8 currently does. We can consider implementing our own `v8::MeasureMemoryDelegate` to select the target context in `ShouldMeasure()` in the future, but then we'll also need to implement `MeasurementComplete()` to assemble the result. For now it's probably too early to do that. Since this API is still experimental (and guarded with a warning), such breakage should be acceptable. Refs: nodejs/node-v8#147 PR-URL: #32988 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Loading branch information
1 parent
4cfa7e0
commit d39254a
Showing
7 changed files
with
226 additions
and
97 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /* eslint-disable node-core/require-common-first, node-core/required-modules */ | ||
| 'use strict'; | ||
|
|
||
| const assert = require('assert'); | ||
| const common = require('./'); | ||
|
|
||
| // The formats could change when V8 is updated, then the tests should be | ||
| // updated accordingly. | ||
| function assertResultShape(result) { | ||
| assert.strictEqual(typeof result.jsMemoryEstimate, 'number'); | ||
| assert.strictEqual(typeof result.jsMemoryRange[0], 'number'); | ||
| assert.strictEqual(typeof result.jsMemoryRange[1], 'number'); | ||
| } | ||
|
|
||
| function assertSummaryShape(result) { | ||
| assert.strictEqual(typeof result, 'object'); | ||
| assert.strictEqual(typeof result.total, 'object'); | ||
| assertResultShape(result.total); | ||
| } | ||
|
|
||
| function assertDetailedShape(result, contexts = 0) { | ||
| assert.strictEqual(typeof result, 'object'); | ||
| assert.strictEqual(typeof result.total, 'object'); | ||
| assert.strictEqual(typeof result.current, 'object'); | ||
| assertResultShape(result.total); | ||
| assertResultShape(result.current); | ||
| if (contexts === 0) { | ||
| assert.deepStrictEqual(result.other, []); | ||
| } else { | ||
| assert.strictEqual(result.other.length, contexts); | ||
| for (const item of result.other) { | ||
| assertResultShape(item); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function assertSingleDetailedShape(result) { | ||
| assert.strictEqual(typeof result, 'object'); | ||
| assert.strictEqual(typeof result.total, 'object'); | ||
| assert.strictEqual(typeof result.current, 'object'); | ||
| assert.deepStrictEqual(result.other, []); | ||
| assertResultShape(result.total); | ||
| assertResultShape(result.current); | ||
| } | ||
|
|
||
| function expectExperimentalWarning() { | ||
| common.expectWarning('ExperimentalWarning', | ||
| 'vm.measureMemory is an experimental feature. ' + | ||
| 'This feature could change at any time'); | ||
| } | ||
|
|
||
| module.exports = { | ||
| assertSummaryShape, | ||
| assertDetailedShape, | ||
| assertSingleDetailedShape, | ||
| expectExperimentalWarning | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Flags: --expose-gc | ||
|
|
||
| 'use strict'; | ||
| const common = require('../common'); | ||
| const { | ||
| assertSummaryShape, | ||
| expectExperimentalWarning | ||
| } = require('../common/measure-memory'); | ||
| const vm = require('vm'); | ||
|
|
||
| expectExperimentalWarning(); | ||
|
|
||
| // Test lazy memory measurement - we will need to global.gc() | ||
| // or otherwise these may not resolve. | ||
| { | ||
| vm.measureMemory() | ||
| .then(common.mustCall(assertSummaryShape)); | ||
| global.gc(); | ||
| } | ||
|
|
||
| { | ||
| vm.measureMemory({}) | ||
| .then(common.mustCall(assertSummaryShape)); | ||
| global.gc(); | ||
| } | ||
|
|
||
| { | ||
| vm.measureMemory({ mode: 'summary' }) | ||
| .then(common.mustCall(assertSummaryShape)); | ||
| global.gc(); | ||
| } | ||
|
|
||
| { | ||
| vm.measureMemory({ mode: 'detailed' }) | ||
| .then(common.mustCall(assertSummaryShape)); | ||
| global.gc(); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| 'use strict'; | ||
| const common = require('../common'); | ||
| const { | ||
| assertDetailedShape, | ||
| expectExperimentalWarning | ||
| } = require('../common/measure-memory'); | ||
| const vm = require('vm'); | ||
| const assert = require('assert'); | ||
|
|
||
| expectExperimentalWarning(); | ||
| { | ||
| const arr = []; | ||
| const count = 10; | ||
| for (let i = 0; i < count; ++i) { | ||
| const context = vm.createContext({ | ||
| test: new Array(100).fill('foo') | ||
| }); | ||
| arr.push(context); | ||
| } | ||
| // Check that one more context shows up in the result | ||
| vm.measureMemory({ mode: 'detailed', execution: 'eager' }) | ||
| .then(common.mustCall((result) => { | ||
| // We must hold on to the contexts here so that they | ||
| // don't get GC'ed until the measurement is complete | ||
| assert.strictEqual(arr.length, count); | ||
| assertDetailedShape(result, count); | ||
| })); | ||
| } |
Oops, something went wrong.