Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: improve networkInterfaces() performance #25410

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions benchmark/os/networkInterfaces.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const common = require('../common.js');
const networkInterfaces = require('os').networkInterfaces;

const bench = common.createBenchmark(main, {
n: [1e4]
});

function main({ n }) {
bench.start();
for (var i = 0; i < n; ++i)
networkInterfaces();
bench.end(n);
}
47 changes: 29 additions & 18 deletions lib/os.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,13 @@ endianness[Symbol.toPrimitive] = () => kEndianness;
// Returns the number of ones in the binary representation of the decimal
// number.
function countBinaryOnes(n) {
let count = 0;
// Remove one "1" bit from n until n is the power of 2. This iterates k times
// while k is the number of "1" in the binary representation.
// For more check https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_Operators
while (n !== 0) {
n = n & (n - 1);
count++;
}
return count;
// Count the number of bits set in parallel, which is faster than looping
n = n - ((n >>> 1) & 0x55555555);
n = (n & 0x33333333) + ((n >>> 2) & 0x33333333);
return ((n + (n >>> 4) & 0xF0F0F0F) * 0x1010101) >>> 24;
}

function getCIDR({ address, netmask, family }) {
function getCIDR(address, netmask, family) {
let ones = 0;
let split = '.';
let range = 10;
Expand Down Expand Up @@ -190,17 +185,33 @@ function getCIDR({ address, netmask, family }) {
}

function networkInterfaces() {
const interfaceAddresses = getInterfaceAddresses();
const data = getInterfaceAddresses();
const result = {};

const keys = Object.keys(interfaceAddresses);
for (var i = 0; i < keys.length; i++) {
const arr = interfaceAddresses[keys[i]];
for (var j = 0; j < arr.length; j++) {
arr[j].cidr = getCIDR(arr[j]);
}
if (data === undefined)
return result;
for (var i = 0; i < data.length; i += 7) {
const name = data[i];
const entry = {
address: data[i + 1],
netmask: data[i + 2],
family: data[i + 3],
mac: data[i + 4],
internal: data[i + 5],
cidr: getCIDR(data[i + 1], data[i + 2], data[i + 3])
};
const scopeid = data[i + 6];
if (scopeid !== -1)
entry.scopeid = scopeid;

const existing = result[name];
if (existing !== undefined)
existing.push(entry);
else
result[name] = [entry];
}
Copy link
Member

@jdalton jdalton Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the perf benefit for networkInterfaces()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Copy link
Member

@jdalton jdalton Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance it looks like a lot of unrolled code with an extra object being created. Could you please step me through the optimization being done in networkInterfaces(). If you drop the rework to networkInterfaces() do you still see perf wins?

Copy link
Contributor Author

@mscdex mscdex Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more or less moving the object creation from C++ to JS land, just like we do for CPU enumeration.


return interfaceAddresses;
return result;
}

function setPriority(pid, priority) {
Expand Down
55 changes: 18 additions & 37 deletions src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,28 +213,28 @@ static void GetLoadAvg(const FunctionCallbackInfo<Value>& args) {

static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();
uv_interface_address_t* interfaces;
int count, i;
char ip[INET6_ADDRSTRLEN];
char netmask[INET6_ADDRSTRLEN];
std::array<char, 18> mac;
Local<Object> ret, o;
Local<String> name, family;
Local<Array> ifarr;

int err = uv_interface_addresses(&interfaces, &count);

ret = Object::New(env->isolate());
if (err == UV_ENOSYS)
return args.GetReturnValue().SetUndefined();

if (err == UV_ENOSYS) {
return args.GetReturnValue().Set(ret);
} else if (err) {
if (err) {
CHECK_GE(args.Length(), 1);
env->CollectUVExceptionInfo(args[args.Length() - 1], errno,
"uv_interface_addresses");
return args.GetReturnValue().SetUndefined();
}

Local<Value> no_scope_id = Integer::New(isolate, -1);
std::vector<Local<Value>> result(count * 7);
for (i = 0; i < count; i++) {
const char* const raw_name = interfaces[i].name;

Expand All @@ -243,17 +243,9 @@ static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
// to assume UTF8 as the default as well. It’s what people will expect if
// they name the interface from any input that uses UTF-8, which should be
// the most frequent case by far these days.)
name = String::NewFromUtf8(env->isolate(), raw_name,
name = String::NewFromUtf8(isolate, raw_name,
v8::NewStringType::kNormal).ToLocalChecked();

if (ret->Has(env->context(), name).FromJust()) {
ifarr = Local<Array>::Cast(ret->Get(env->context(),
name).ToLocalChecked());
} else {
ifarr = Array::New(env->isolate());
ret->Set(env->context(), name, ifarr).FromJust();
}

snprintf(mac.data(),
mac.size(),
"%02x:%02x:%02x:%02x:%02x:%02x",
Expand All @@ -277,34 +269,23 @@ static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
family = env->unknown_string();
}

o = Object::New(env->isolate());
o->Set(env->context(),
env->address_string(),
OneByteString(env->isolate(), ip)).FromJust();
o->Set(env->context(),
env->netmask_string(),
OneByteString(env->isolate(), netmask)).FromJust();
o->Set(env->context(),
env->family_string(), family).FromJust();
o->Set(env->context(),
env->mac_string(),
FIXED_ONE_BYTE_STRING(env->isolate(), mac)).FromJust();

result[i * 7] = name;
result[i * 7 + 1] = OneByteString(isolate, ip);
result[i * 7 + 2] = OneByteString(isolate, netmask);
result[i * 7 + 3] = family;
result[i * 7 + 4] = FIXED_ONE_BYTE_STRING(isolate, mac);
result[i * 7 + 5] =
interfaces[i].is_internal ? True(isolate) : False(isolate);
if (interfaces[i].address.address4.sin_family == AF_INET6) {
uint32_t scopeid = interfaces[i].address.address6.sin6_scope_id;
o->Set(env->context(), env->scopeid_string(),
Integer::NewFromUnsigned(env->isolate(), scopeid)).FromJust();
result[i * 7 + 6] = Integer::NewFromUnsigned(isolate, scopeid);
} else {
result[i * 7 + 6] = no_scope_id;
}

const bool internal = interfaces[i].is_internal;
o->Set(env->context(), env->internal_string(),
internal ? True(env->isolate()) : False(env->isolate())).FromJust();

ifarr->Set(env->context(), ifarr->Length(), o).FromJust();
}

uv_free_interface_addresses(interfaces, count);
args.GetReturnValue().Set(ret);
args.GetReturnValue().Set(Array::New(isolate, result.data(), result.size()));
}


Expand Down