Skip to content

Feature/faster crypto#664

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.

Sign up for GitHub

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

Closed
indutny wants to merge 5 commits into from
Closed

Feature/faster crypto #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c319fd1
node: speed up ParseEncoding
indutny Jan 30, 2015
38cf628
crypto: use on-stack storage in HashUpdate
indutny Jan 30, 2015
fcbf80c
string_bytes: introduce InlineDecoder
indutny Jan 30, 2015
077bc99
crypto: use StringBytes::InlineDecoder
indutny Jan 30, 2015
7c05132
node: ParseEncoding fix
indutny Jan 30, 2015
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
83 changes: 61 additions & 22 deletions src/node.cc
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1148,45 +1148,72 @@ Handle<Value> MakeCallback(Isolate* isolate,
}


enum encoding ParseEncoding(Isolate* isolate,
Handle<Value> encoding_v,
enum encoding _default) {
HandleScope scope(isolate);

if (!encoding_v->IsString())
return _default;

node::Utf8Value encoding(isolate, encoding_v);
enum encoding ParseEncoding(const char* encoding, enum encoding _default) {
switch (encoding[0]) {
case 'u':
// utf8, utf16le
if (encoding[1] == 't' && encoding[2] == 'f') {
// Skip `-`
encoding += encoding[3] == '-' ? 4 : 3;
if (encoding[0] == '8' && encoding[1] == '\0')
return UTF8;
if (strncmp(encoding, "16le", 4) == 0)
return UCS2;

// ucs2
} else if (encoding[1] == 'c' && encoding[2] == 's') {
encoding += encoding[3] == '-' ? 4 : 3;
if (encoding[0] == '2' && encoding[1] == '\0')
return UCS2;
}
break;
case 'b':
// binary
if (encoding[1] == 'i') {
if (strncmp(encoding + 2, "nary", 4) == 0)
return BINARY;

// buffer
} else if (encoding[1] == 'u') {
if (strncmp(encoding + 2, "ffer", 4) == 0)
return BUFFER;
}
break;
case '\0':
return _default;
default:
break;
}

if (strcasecmp(*encoding, "utf8") == 0) {
if (strcasecmp(encoding, "utf8") == 0) {
return UTF8;
} else if (strcasecmp(*encoding, "utf-8") == 0) {
} else if (strcasecmp(encoding, "utf-8") == 0) {
return UTF8;
} else if (strcasecmp(*encoding, "ascii") == 0) {
} else if (strcasecmp(encoding, "ascii") == 0) {
return ASCII;
} else if (strcasecmp(*encoding, "base64") == 0) {
} else if (strcasecmp(encoding, "base64") == 0) {
return BASE64;
} else if (strcasecmp(*encoding, "ucs2") == 0) {
} else if (strcasecmp(encoding, "ucs2") == 0) {
return UCS2;
} else if (strcasecmp(*encoding, "ucs-2") == 0) {
} else if (strcasecmp(encoding, "ucs-2") == 0) {
return UCS2;
} else if (strcasecmp(*encoding, "utf16le") == 0) {
} else if (strcasecmp(encoding, "utf16le") == 0) {
return UCS2;
} else if (strcasecmp(*encoding, "utf-16le") == 0) {
} else if (strcasecmp(encoding, "utf-16le") == 0) {
return UCS2;
} else if (strcasecmp(*encoding, "binary") == 0) {
} else if (strcasecmp(encoding, "binary") == 0) {
return BINARY;
} else if (strcasecmp(*encoding, "buffer") == 0) {
} else if (strcasecmp(encoding, "buffer") == 0) {
return BUFFER;
} else if (strcasecmp(*encoding, "hex") == 0) {
} else if (strcasecmp(encoding, "hex") == 0) {
return HEX;
} else if (strcasecmp(*encoding, "raw") == 0) {
} else if (strcasecmp(encoding, "raw") == 0) {
if (!no_deprecation) {
fprintf(stderr, "'raw' (array of integers) has been removed. "
"Use 'binary'.\n");
}
return BINARY;
} else if (strcasecmp(*encoding, "raws") == 0) {
} else if (strcasecmp(encoding, "raws") == 0) {
if (!no_deprecation) {
fprintf(stderr, "'raws' encoding has been renamed to 'binary'. "
"Please update your code.\n");
Expand All@@ -1197,6 +1224,18 @@ enum encoding ParseEncoding(Isolate* isolate,
}
}


enum encoding ParseEncoding(Isolate* isolate,
Handle<Value> encoding_v,
enum encoding _default) {
if (!encoding_v->IsString())
return _default;

node::Utf8Value encoding(isolate, encoding_v);

return ParseEncoding(*encoding, _default);

@bnoordhuisbnoordhuisJan 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can probably do a little better still by also passing encoding->length(). Then you can filter on length first before you start comparing bytes.

@bnoordhuisbnoordhuisJan 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, wouldn't it be cheaper to do the comparison in JS and just pass integers to C++ land? I suspect that the biggest overhead is from the new[]/delete[] calls, not the string comparison.

@indutnyindutnyJan 30, 2015

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'm +1 for JS-land thing, but for now - let's do it the way it is. encoding->length() doesn't seem to be improving situation much on my benchmarks.

}

Local<Value> Encode(Isolate* isolate,
const char* buf,
size_t len,
Expand Down
90 changes: 25 additions & 65 deletions src/node_crypto.cc
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2750,19 +2750,11 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {

// Only copy the data if we have to, because it's a string
if (args[0]->IsString()) {
Local<String> string = args[0].As<String>();
enum encoding encoding = ParseEncoding(env->isolate(), args[1], BINARY);
if (!StringBytes::IsValidString(env->isolate(), string, encoding))
return env->ThrowTypeError("Bad input string");
size_t buflen = StringBytes::StorageSize(env->isolate(), string, encoding);
char* buf = new char[buflen];
size_t written = StringBytes::Write(env->isolate(),
buf,
buflen,
string,
encoding);
r = cipher->Update(buf, written, &out, &out_len);
delete[] buf;
StringBytes::InlineDecoder decoder(env);

if (!decoder.Decode(args[0].As<String>(), args[1], BINARY))
return;
r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len);
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
Expand DownExpand Up@@ -2929,19 +2921,11 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
bool r;
if (args[0]->IsString()) {
Local<String> string = args[0].As<String>();
enum encoding encoding = ParseEncoding(env->isolate(), args[1], BINARY);
if (!StringBytes::IsValidString(env->isolate(), string, encoding))
return env->ThrowTypeError("Bad input string");
size_t buflen = StringBytes::StorageSize(env->isolate(), string, encoding);
char* buf = new char[buflen];
size_t written = StringBytes::Write(env->isolate(),
buf,
buflen,
string,
encoding);
r = hmac->HmacUpdate(buf, written);
delete[] buf;
StringBytes::InlineDecoder decoder(env);

if (!decoder.Decode(args[0].As<String>(), args[1], BINARY))
return;
r = hmac->HmacUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
Expand DownExpand Up@@ -3053,19 +3037,11 @@ void Hash::HashUpdate(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
bool r;
if (args[0]->IsString()) {
Local<String> string = args[0].As<String>();
enum encoding encoding = ParseEncoding(env->isolate(), args[1], BINARY);
if (!StringBytes::IsValidString(env->isolate(), string, encoding))
return env->ThrowTypeError("Bad input string");
size_t buflen = StringBytes::StorageSize(env->isolate(), string, encoding);
char* buf = new char[buflen];
size_t written = StringBytes::Write(env->isolate(),
buf,
buflen,
string,
encoding);
r = hash->HashUpdate(buf, written);
delete[] buf;
StringBytes::InlineDecoder decoder(env);

if (!decoder.Decode(args[0].As<String>(), args[1], BINARY))
return;
r = hash->HashUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
Expand DownExpand Up@@ -3214,19 +3190,11 @@ void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
Error err;
if (args[0]->IsString()) {
Local<String> string = args[0].As<String>();
enum encoding encoding = ParseEncoding(env->isolate(), args[1], BINARY);
if (!StringBytes::IsValidString(env->isolate(), string, encoding))
return env->ThrowTypeError("Bad input string");
size_t buflen = StringBytes::StorageSize(env->isolate(), string, encoding);
char* buf = new char[buflen];
size_t written = StringBytes::Write(env->isolate(),
buf,
buflen,
string,
encoding);
err = sign->SignUpdate(buf, written);
delete[] buf;
StringBytes::InlineDecoder decoder(env);

if (!decoder.Decode(args[0].As<String>(), args[1], BINARY))
return;
err = sign->SignUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
Expand DownExpand Up@@ -3395,19 +3363,11 @@ void Verify::VerifyUpdate(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
Error err;
if (args[0]->IsString()) {
Local<String> string = args[0].As<String>();
enum encoding encoding = ParseEncoding(env->isolate(), args[1], BINARY);
if (!StringBytes::IsValidString(env->isolate(), string, encoding))
return env->ThrowTypeError("Bad input string");
size_t buflen = StringBytes::StorageSize(env->isolate(), string, encoding);
char* buf = new char[buflen];
size_t written = StringBytes::Write(env->isolate(),
buf,
buflen,
string,
encoding);
err = verify->VerifyUpdate(buf, written);
delete[] buf;
StringBytes::InlineDecoder decoder(env);

if (!decoder.Decode(args[0].As<String>(), args[1], BINARY))
return;
err = verify->VerifyUpdate(decoder.out(), decoder.size());
} else {
char* buf = Buffer::Data(args[0]);
size_t buflen = Buffer::Length(args[0]);
Expand Down
47 changes: 47 additions & 0 deletions src/string_bytes.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,13 +5,60 @@

#include "v8.h"
#include "node.h"
#include "env.h"
#include "env-inl.h"

namespace node {

extern int WRITE_UTF8_FLAGS;

class StringBytes {
public:
class InlineDecoder {
public:
explicit InlineDecoder(Environment* env) : env_(env), out_(nullptr) {
}

~InlineDecoder() {
if (out_ != out_st_)
delete[] out_;
out_ = nullptr;
}

inline bool Decode(v8::Handle<v8::String> string,
v8::Handle<v8::Value> encoding,
enum encoding _default) {

@bnoordhuisbnoordhuisJan 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a tiny nit and I realize you copied the convention from src/node.cc, but _default is a horrible name for a parameter. Something like default_encoding would be better.

@indutnyindutnyJan 30, 2015

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Changed.

enum encoding enc = ParseEncoding(env_->isolate(), encoding, _default);
if (!StringBytes::IsValidString(env_->isolate(), string, enc)) {
env_->ThrowTypeError("Bad input string");
return false;
}

size_t buflen = StringBytes::StorageSize(env_->isolate(), string, enc);
if (buflen > sizeof(out_st_))
out_ = new char[buflen];
else
out_ = out_st_;
size_ = StringBytes::Write(env_->isolate(),
out_,
buflen,
string,
enc);
return true;
}

inline const char* out() const { return out_; }
inline size_t size() const { return size_; }

private:
static const int kStorageSize = 1024;

Environment* env_;

@bnoordhuisbnoordhuisJan 30, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe pass the env as an argument to Decode()? It's not a big win but it reduces the stack footprint by a word.

@indutnyindutnyJan 30, 2015

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Makes sense.

char out_st_[kStorageSize];
char* out_;
size_t size_;
};

// Does the string match the encoding? Quick but non-exhaustive.
// Example: a HEX string must have a length that's a multiple of two.
// FIXME(bnoordhuis) IsMaybeValidString()? Naming things is hard...
Expand Down