Skip to content

Commit a8c3d03

Browse files
committed
src: fix negative nodeTiming milestone values
PR-URL: #46588 Fixes: #45958 Refs: #43781 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 53116b3 commit a8c3d03

File tree

6 files changed

+73
-12
lines changed

6 files changed

+73
-12
lines changed

src/env.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,9 @@ Environment::Environment(IsolateData* isolate_data,
671671
stream_base_state_(isolate_,
672672
StreamBase::kNumStreamBaseStateFields,
673673
MAYBE_FIELD_PTR(env_info, stream_base_state)),
674-
time_origin_(PERFORMANCE_NOW()),
675-
time_origin_timestamp_(GetCurrentTimeInMicroseconds()),
674+
time_origin_(performance::performance_process_start),
675+
time_origin_timestamp_(performance::performance_process_start_timestamp),
676+
environment_start_(PERFORMANCE_NOW()),
676677
flags_(flags),
677678
thread_id_(thread_id.id == static_cast<uint64_t>(-1)
678679
? AllocateEnvironmentThreadId().id
@@ -803,7 +804,7 @@ void Environment::InitializeMainContext(Local<Context> context,
803804
set_exiting(false);
804805

805806
performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_ENVIRONMENT,
806-
time_origin_);
807+
environment_start_);
807808
performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_NODE_START,
808809
per_process::node_start_time);
809810

src/env.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -846,9 +846,11 @@ class Environment : public MemoryRetainer {
846846
inline HandleWrapQueue* handle_wrap_queue() { return &handle_wrap_queue_; }
847847
inline ReqWrapQueue* req_wrap_queue() { return &req_wrap_queue_; }
848848

849+
// https://w3c.github.io/hr-time/#dfn-time-origin
849850
inline uint64_t time_origin() {
850851
return time_origin_;
851852
}
853+
// https://w3c.github.io/hr-time/#dfn-get-time-origin-timestamp
852854
inline double time_origin_timestamp() {
853855
return time_origin_timestamp_;
854856
}
@@ -1057,10 +1059,17 @@ class Environment : public MemoryRetainer {
10571059

10581060
AliasedInt32Array stream_base_state_;
10591061

1060-
// https://w3c.github.io/hr-time/#dfn-time-origin
1061-
uint64_t time_origin_;
1062-
// https://w3c.github.io/hr-time/#dfn-get-time-origin-timestamp
1063-
double time_origin_timestamp_;
1062+
// As PerformanceNodeTiming is exposed in worker_threads, the per_process
1063+
// time origin is exposed in the worker threads. This is an intentional
1064+
// diverge from the HTML spec of web workers.
1065+
// Process start time from the monotonic clock. This should not be used as an
1066+
// absolute time, but only as a time relative to another monotonic clock time.
1067+
const uint64_t time_origin_;
1068+
// Process start timestamp from the wall clock. This is an absolute time
1069+
// exposed as `performance.timeOrigin`.
1070+
const double time_origin_timestamp_;
1071+
// This is the time when the environment is created.
1072+
const uint64_t environment_start_;
10641073
std::unique_ptr<performance::PerformanceState> performance_state_;
10651074

10661075
bool has_serialized_options_ = false;

src/node_perf.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ using v8::Value;
3838
// Nanoseconds in a millisecond, as a float.
3939
#define NANOS_PER_MILLIS 1e6
4040

41+
const uint64_t performance_process_start = PERFORMANCE_NOW();
42+
const double performance_process_start_timestamp =
43+
GetCurrentTimeInMicroseconds();
4144
uint64_t performance_v8_start;
4245

4346
PerformanceState::PerformanceState(Isolate* isolate,
@@ -271,7 +274,7 @@ void CreateELDHistogram(const FunctionCallbackInfo<Value>& args) {
271274
void GetTimeOrigin(const FunctionCallbackInfo<Value>& args) {
272275
Environment* env = Environment::GetCurrent(args);
273276
args.GetReturnValue().Set(
274-
Number::New(args.GetIsolate(), env->time_origin() / 1e6));
277+
Number::New(args.GetIsolate(), env->time_origin() / NANOS_PER_MILLIS));
275278
}
276279

277280
void GetTimeOriginTimeStamp(const FunctionCallbackInfo<Value>& args) {

src/node_perf_common.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ namespace performance {
2020

2121
// These occur before the environment is created. Cache them
2222
// here and add them to the milestones when the env is init'd.
23+
extern const uint64_t performance_process_start;
24+
extern const double performance_process_start_timestamp;
2325
extern uint64_t performance_v8_start;
2426

2527
#define NODE_PERFORMANCE_MILESTONES(V) \

test/parallel/test-perf-hooks-worker-timeorigin.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ require('worker_threads').parentPort.postMessage(performance.timeOrigin);
99
`, { eval: true });
1010

1111
w.on('message', common.mustCall((timeOrigin) => {
12-
// Worker is created after this main context, it's
13-
// `performance.timeOrigin` must be greater than this
14-
// main context's.
15-
assert.ok(timeOrigin > performance.timeOrigin);
12+
// PerformanceNodeTiming exposes process milestones so the
13+
// `performance.timeOrigin` in the `worker_threads.Worker` must be the start
14+
// time of the process.
15+
assert.strictEqual(timeOrigin, performance.timeOrigin);
1616
}));
1717

1818
w.on('exit', common.mustCall((code) => {
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { performance } = require('perf_hooks');
6+
const { isMainThread } = require('worker_threads');
7+
8+
const { nodeTiming } = performance;
9+
assert.strictEqual(nodeTiming.name, 'node');
10+
assert.strictEqual(nodeTiming.entryType, 'node');
11+
12+
assert.strictEqual(nodeTiming.startTime, 0);
13+
const now = performance.now();
14+
assert.ok(nodeTiming.duration >= now);
15+
16+
// Check that the nodeTiming milestone values are in the correct order and greater than 0.
17+
const keys = ['nodeStart', 'v8Start', 'environment', 'bootstrapComplete'];
18+
for (let idx = 0; idx < keys.length; idx++) {
19+
if (idx === 0) {
20+
assert.ok(nodeTiming[keys[idx]] >= 0);
21+
continue;
22+
}
23+
assert.ok(nodeTiming[keys[idx]] > nodeTiming[keys[idx - 1]], `expect nodeTiming['${keys[idx]}'] > nodeTiming['${keys[idx - 1]}']`);
24+
}
25+
26+
// loop milestones.
27+
assert.strictEqual(nodeTiming.idleTime, 0);
28+
if (isMainThread) {
29+
// Main thread does not start loop until the first tick is finished.
30+
assert.strictEqual(nodeTiming.loopStart, -1);
31+
} else {
32+
// Worker threads run the user script after loop is started.
33+
assert.ok(nodeTiming.loopStart >= nodeTiming.bootstrapComplete);
34+
}
35+
assert.strictEqual(nodeTiming.loopExit, -1);
36+
37+
setTimeout(common.mustCall(() => {
38+
assert.ok(nodeTiming.idleTime >= 0);
39+
assert.ok(nodeTiming.idleTime + nodeTiming.loopExit <= nodeTiming.duration);
40+
assert.ok(nodeTiming.loopStart >= nodeTiming.bootstrapComplete);
41+
}, 1), 1);
42+
43+
// Can not be wrapped in common.mustCall().
44+
process.on('exit', () => {
45+
assert.ok(nodeTiming.loopExit > 0);
46+
});

0 commit comments

Comments
 (0)