Skip to content

Commit

Permalink
async_wrap: add uid to TickObject instances
Browse files Browse the repository at this point in the history 
So that each nextTick callback is treated as its own asynchronous event,
add a unique id to new TickObject instances. This is a preliminary for
allowing the async wrap callbacks to run with nextTick callbacks.
  • Loading branch information
@trevnorris
trevnorris committed Apr 8, 2016
1 parent b11d031 commit 4155f0e
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 7 deletions.
24 changes: 24 additions & 0 deletions lib/internal/async_wrap.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const async_wrap = process.binding('async_wrap');
const uidArray = async_wrap.getUidArray();

module.exports.getUid =
process.binding('os').isBigEndian ? getUidBE : getUidLE;


function getUidLE() {
if (uidArray[0] === 0xffffffff) {
uidArray[0] = 0;
uidArray[1]++;

This comment has been minimized.

Sign in to view

Copy link
@mike-kaufman

mike-kaufman Apr 13, 2016

@trevnorris - is this ++ intended to increment the value? And is this expected to update the memory in the runtime?

This comment has been minimized.

Sign in to view

Copy link
@trevnorris

trevnorris Apr 14, 2016

Author Owner

yes and yes. though it seems I missed the else { uidArray[0]++; } after this.

This comment has been minimized.

Sign in to view

Copy link
@mike-kaufman

mike-kaufman Apr 14, 2016

I'm not following why you're testing for the lsb == 0xffffffff and then setting to zero? Are you trying to convert from 2's complement?

This comment has been minimized.

Sign in to view

Copy link
@trevnorris

trevnorris Apr 14, 2016

Author Owner

This array holds the upper and lower 32 bits of an int64_t. But if the lower 32 bits is incremented in this way it will loop over and hit zero. So we need to manually increment the upper 32 bits if the lower is about to flip.

Reason for using an int64_t is because Number.MAX_SAFE_INTEGER is 2^53 - 1. Which gives us comfortable breathing room for assigning unique id's without needing to worry that we'll run out.

This comment has been minimized.

Sign in to view

Copy link
@trevnorris

trevnorris Apr 14, 2016

Author Owner

Example using typed arrays:

var ab = new ArrayBuffer(4);
var i32 = new Int32Array(ab);
var u16 = new Uint16Array(ab);
u16[0] = 0xffff;
i32[0] == 0xffff;
u16[0]++;
i32[0] == 0;
u16[1]++;
i32[0] = 0x10000;

Same thing is happening here, except we're managing a 64 bit integer so we can quickly grab the 2^53 - 1 parts without transversing to C++.

This comment has been minimized.

Sign in to view

Copy link
@mike-kaufman

mike-kaufman Apr 14, 2016

I see, you're doing carry-over to the MSB. So correct logic for reading big-endian values is:

function readBigEndian(uid) {
    return uid[1] + uid[0] * 0x100000000;
}

and correct logic for increment big-endian values is:

function incrementBigEndian(uid) {
  if (uid[1] === 0xffffffff) {
    uid[1] = 0;
    uid[0]++;
  }
  uid[1]++; 
  return uid[1] + uid[0] * 0x100000000
}

This comment has been minimized.

Sign in to view

Copy link
@mike-kaufman

mike-kaufman Apr 14, 2016 •
edited
Loading

Reason for using an int64_t is because Number.MAX_SAFE_INTEGER is 2^53 - 1

Note that the int64_t uids are being converted to an int32_t values throughout the runtime whenever the uids are passed to registered callbacks.. Moreover, the logic above is subject to overflow if the value goes beyond 53 bits. See nodejs/diagnostics#44 for an issue I opened on this.

This comment has been minimized.

Sign in to view

Copy link
@trevnorris

trevnorris Apr 14, 2016

Author Owner

I'm aware, and have a fix done and in the pipe for my next asyncwrap PR. I'm genuinely not worried about overflow. Even incrementing the value every 100ns we still have 28 years.
}
return uidArray[0] + uidArray[1] * 0x100000000;
}

function getUidBE() {
if (uidArray[1] === 0xffffffff) {
uidArray[1] = 0;
uidArray[0]++;
}
return uidArray[1] + uidArray[0] * 0x100000000;
}
2 changes: 2 additions & 0 deletions lib/internal/process/next_tick.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports.setup = setupNextTick;

function setupNextTick() {
const async_wrap = require('internal/async_wrap');
const promises = require('internal/process/promises');
const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks);
var nextTickQueue = [];
Expand Down Expand Up @@ -135,6 +136,7 @@ function setupNextTick() {
this.callback = c;
this.domain = process.domain || null;
this.args = args;
this.uid = async_wrap.getUid();
}

function nextTick(callback) {
Expand Down
1 change: 1 addition & 0 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
'lib/v8.js',
'lib/vm.js',
'lib/zlib.js',
'lib/internal/async_wrap.js',
'lib/internal/child_process.js',
'lib/internal/cluster.js',
'lib/internal/freelist.js',
Expand Down
11 changes: 9 additions & 2 deletions src/async-wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,20 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
}


static void GetUidArray(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
args.GetReturnValue().Set(env->async_hooks()->get_uid_array());
}


static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context) {
Local<Value> unused,
Local<Context> context) {
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();
HandleScope scope(isolate);

env->SetMethod(target, "getUidArray", GetUidArray);
env->SetMethod(target, "setupHooks", SetupHooks);
env->SetMethod(target, "disable", DisableHooksJS);
env->SetMethod(target, "enable", EnableHooksJS);
Expand Down
24 changes: 21 additions & 3 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,18 @@ inline v8::Isolate* Environment::IsolateData::isolate() const {
return isolate_;
}

inline Environment::AsyncHooks::AsyncHooks() {
inline Environment::AsyncHooks::AsyncHooks(Environment* env)
: async_wrap_uid_(0),
env_(env) {
for (int i = 0; i < kFieldsCount; i++) fields_[i] = 0;
const size_t array_length = sizeof(async_wrap_uid_) / sizeof(int32_t);
static_assert(array_length == 2, "async_wrap_uid_ unexpected size");
v8::HandleScope handle_scope(env_->isolate());
v8::Local<v8::ArrayBuffer> ab =
v8::ArrayBuffer::New(env_->isolate(), &async_wrap_uid_, array_length);
v8::Local<v8::Uint32Array> ua =
v8::Uint32Array::New(ab, 0, array_length);
async_wrap_uid_array_.Reset(env_->isolate(), ua);
}

inline uint32_t* Environment::AsyncHooks::fields() {
Expand All @@ -101,6 +111,14 @@ inline void Environment::AsyncHooks::set_enable_callbacks(uint32_t flag) {
fields_[kEnableCallbacks] = flag;
}

inline int64_t Environment::AsyncHooks::get_async_wrap_uid() {
return ++async_wrap_uid_;
}

inline v8::Local<v8::Uint32Array> Environment::AsyncHooks::get_uid_array() {
return async_wrap_uid_array_.Get(env_->isolate());
}

inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
: env_(env) {
env_->makecallback_cntr_++;
Expand Down Expand Up @@ -216,12 +234,12 @@ inline Environment::Environment(v8::Local<v8::Context> context,
uv_loop_t* loop)
: isolate_(context->GetIsolate()),
isolate_data_(IsolateData::GetOrCreate(context->GetIsolate(), loop)),
async_hooks_(this),
timer_base_(uv_now(loop)),
using_domains_(false),
printed_error_(false),
trace_sync_io_(false),
makecallback_cntr_(0),
async_wrap_uid_(0),
debugger_agent_(this),
http_parser_buffer_(nullptr),
context_(context->GetIsolate(), context) {
Expand Down Expand Up @@ -373,7 +391,7 @@ inline void Environment::set_trace_sync_io(bool value) {
}

inline int64_t Environment::get_async_wrap_uid() {
return ++async_wrap_uid_;
return async_hooks()->get_async_wrap_uid();
}

inline uint32_t* Environment::heap_statistics_buffer() const {
Expand Down
8 changes: 6 additions & 2 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,13 @@ class Environment {
inline int fields_count() const;
inline bool callbacks_enabled();
inline void set_enable_callbacks(uint32_t flag);
inline int64_t get_async_wrap_uid();
inline v8::Local<v8::Uint32Array> get_uid_array();

private:
friend class Environment; // So we can call the constructor.
inline AsyncHooks();
inline AsyncHooks(Environment* env);
v8::Persistent<v8::Uint32Array> async_wrap_uid_array_;

enum Fields {
// Set this to not zero if the init hook should be called.
Expand All @@ -312,6 +315,8 @@ class Environment {
};

uint32_t fields_[kFieldsCount];
int64_t async_wrap_uid_;
Environment* env_;

DISALLOW_COPY_AND_ASSIGN(AsyncHooks);
};
Expand Down Expand Up @@ -577,7 +582,6 @@ class Environment {
bool printed_error_;
bool trace_sync_io_;
size_t makecallback_cntr_;
int64_t async_wrap_uid_;
debugger::Agent debugger_agent_;

HandleWrapQueue handle_wrap_queue_;
Expand Down

0 comments on commit 4155f0e

Please sign in to comment.