Skip to content

Commit

Permalink
src: migrate off ArrayBuffer::GetContents
Browse files Browse the repository at this point in the history
V8 deprecates `GetContents()` in favour of `GetBackingStore()`.
Update our code to reflect that.
V8 also deprecates `Externalize()` and `IsExternal()`; we should
be able to remove all usage of this once V8 8.0 is there.

PR-URL: #30339
Refs: v8/v8@bfe3d6b
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: David Carlier <[email protected]>
  • Loading branch information
addaleax authored and MylesBorins committed Nov 21, 2019
1 parent f17c794 commit 1a92c88
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class AliasedBufferBase {
// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, size_in_bytes);
buffer_ = static_cast<NativeT*>(ab->GetContents().Data());
buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
Expand Down Expand Up @@ -228,7 +228,7 @@ class AliasedBufferBase {
isolate_, new_size_in_bytes);

// allocate new native buffer
NativeT* new_buffer = static_cast<NativeT*>(ab->GetContents().Data());
NativeT* new_buffer = static_cast<NativeT*>(ab->GetBackingStore()->Data());
// copy old content
memcpy(new_buffer, buffer_, old_size_in_bytes);

Expand Down
31 changes: 22 additions & 9 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2562,7 +2562,7 @@ napi_status napi_create_arraybuffer(napi_env env,
// Optionally return a pointer to the buffer's data, to avoid another call to
// retrieve it.
if (data != nullptr) {
*data = buffer->GetContents().Data();
*data = buffer->GetBackingStore()->Data();
}

*result = v8impl::JsValueFromV8LocalValue(buffer);
Expand Down Expand Up @@ -2608,15 +2608,15 @@ napi_status napi_get_arraybuffer_info(napi_env env,
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);

v8::ArrayBuffer::Contents contents =
value.As<v8::ArrayBuffer>()->GetContents();
std::shared_ptr<v8::BackingStore> backing_store =
value.As<v8::ArrayBuffer>()->GetBackingStore();

if (data != nullptr) {
*data = contents.Data();
*data = backing_store->Data();
}

if (byte_length != nullptr) {
*byte_length = contents.ByteLength();
*byte_length = backing_store->ByteLength();
}

return napi_clear_last_error(env);
Expand Down Expand Up @@ -2747,9 +2747,15 @@ napi_status napi_get_typedarray_info(napi_env env,
*length = array->Length();
}

v8::Local<v8::ArrayBuffer> buffer = array->Buffer();
v8::Local<v8::ArrayBuffer> buffer;
if (data != nullptr || arraybuffer != nullptr) {
// Calling Buffer() may have the side effect of allocating the buffer,
// so only do this when it’s needed.
buffer = array->Buffer();
}

if (data != nullptr) {
*data = static_cast<uint8_t*>(buffer->GetContents().Data()) +
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
array->ByteOffset();
}

Expand Down Expand Up @@ -2821,9 +2827,15 @@ napi_status napi_get_dataview_info(napi_env env,
*byte_length = array->ByteLength();
}

v8::Local<v8::ArrayBuffer> buffer = array->Buffer();
v8::Local<v8::ArrayBuffer> buffer;
if (data != nullptr || arraybuffer != nullptr) {
// Calling Buffer() may have the side effect of allocating the buffer,
// so only do this when it’s needed.
buffer = array->Buffer();
}

if (data != nullptr) {
*data = static_cast<uint8_t*>(buffer->GetContents().Data()) +
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
array->ByteOffset();
}

Expand Down Expand Up @@ -3015,6 +3027,7 @@ napi_status napi_detach_arraybuffer(napi_env env, napi_value arraybuffer) {
env, value->IsArrayBuffer(), napi_arraybuffer_expected);

v8::Local<v8::ArrayBuffer> it = value.As<v8::ArrayBuffer>();
// TODO(addaleax): Remove the first condition once we have V8 8.0.
RETURN_STATUS_IF_FALSE(
env, it->IsExternal(), napi_detachable_arraybuffer_expected);
RETURN_STATUS_IF_FALSE(
Expand Down
13 changes: 5 additions & 8 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,13 @@ bool HasInstance(Local<Object> obj) {
char* Data(Local<Value> val) {
CHECK(val->IsArrayBufferView());
Local<ArrayBufferView> ui = val.As<ArrayBufferView>();
ArrayBuffer::Contents ab_c = ui->Buffer()->GetContents();
return static_cast<char*>(ab_c.Data()) + ui->ByteOffset();
return static_cast<char*>(ui->Buffer()->GetBackingStore()->Data()) +
ui->ByteOffset();
}


char* Data(Local<Object> obj) {
CHECK(obj->IsArrayBufferView());
Local<ArrayBufferView> ui = obj.As<ArrayBufferView>();
ArrayBuffer::Contents ab_c = ui->Buffer()->GetContents();
return static_cast<char*>(ab_c.Data()) + ui->ByteOffset();
return Data(obj.As<Value>());
}


Expand Down Expand Up @@ -1060,13 +1057,13 @@ static void EncodeInto(const FunctionCallbackInfo<Value>& args) {
Local<Uint8Array> dest = args[1].As<Uint8Array>();
Local<ArrayBuffer> buf = dest->Buffer();
char* write_result =
static_cast<char*>(buf->GetContents().Data()) + dest->ByteOffset();
static_cast<char*>(buf->GetBackingStore()->Data()) + dest->ByteOffset();
size_t dest_length = dest->ByteLength();

// results = [ read, written ]
Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
uint32_t* results = reinterpret_cast<uint32_t*>(
static_cast<char*>(result_arr->Buffer()->GetContents().Data()) +
static_cast<char*>(result_arr->Buffer()->GetBackingStore()->Data()) +
result_arr->ByteOffset());

int nchars;
Expand Down
8 changes: 4 additions & 4 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -697,8 +697,8 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {

ScriptCompiler::CachedData* cached_data = nullptr;
if (!cached_data_buf.IsEmpty()) {
ArrayBuffer::Contents contents = cached_data_buf->Buffer()->GetContents();
uint8_t* data = static_cast<uint8_t*>(contents.Data());
uint8_t* data = static_cast<uint8_t*>(
cached_data_buf->Buffer()->GetBackingStore()->Data());
cached_data = new ScriptCompiler::CachedData(
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
}
Expand Down Expand Up @@ -1044,8 +1044,8 @@ void ContextifyContext::CompileFunction(
// Read cache from cached data buffer
ScriptCompiler::CachedData* cached_data = nullptr;
if (!cached_data_buf.IsEmpty()) {
ArrayBuffer::Contents contents = cached_data_buf->Buffer()->GetContents();
uint8_t* data = static_cast<uint8_t*>(contents.Data());
uint8_t* data = static_cast<uint8_t*>(
cached_data_buf->Buffer()->GetBackingStore()->Data());
cached_data = new ScriptCompiler::CachedData(
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
}
Expand Down
21 changes: 15 additions & 6 deletions src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,19 @@ Maybe<bool> Message::Serialize(Environment* env,
// Currently, we support ArrayBuffers and MessagePorts.
if (entry->IsArrayBuffer()) {
Local<ArrayBuffer> ab = entry.As<ArrayBuffer>();
// If we cannot render the ArrayBuffer unusable in this Isolate and
// take ownership of its memory, copying the buffer will have to do.
if (!ab->IsDetachable() || ab->IsExternal() ||
!env->isolate_data()->uses_node_allocator()) {
// If we cannot render the ArrayBuffer unusable in this Isolate,
// copying the buffer will have to do.
// Note that we can currently transfer ArrayBuffers even if they were
// not allocated by Node’s ArrayBufferAllocator in the first place,
// because we pass the underlying v8::BackingStore around rather than
// raw data *and* an Isolate with a non-default ArrayBuffer allocator
// is always going to outlive any Workers it creates, and so will its
// allocator along with it.
// TODO(addaleax): Eventually remove the IsExternal() condition,
// see https://github.com/nodejs/node/pull/30339#issuecomment-552225353
// for details.
if (!ab->IsDetachable() || ab->IsExternal())
continue;
}
if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
array_buffers.end()) {
ThrowDataCloneException(
Expand Down Expand Up @@ -362,7 +369,9 @@ Maybe<bool> Message::Serialize(Environment* env,
for (Local<ArrayBuffer> ab : array_buffers) {
// If serialization succeeded, we render it inaccessible in this Isolate.
std::shared_ptr<BackingStore> backing_store = ab->GetBackingStore();
ab->Externalize(backing_store);
// TODO(addaleax): This can/should be dropped once we have V8 8.0.
if (!ab->IsExternal())
ab->Externalize(backing_store);
ab->Detach();

array_buffers_.emplace_back(std::move(backing_store));
Expand Down
2 changes: 1 addition & 1 deletion src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static void GetLoadAvg(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 3);
Local<ArrayBuffer> ab = array->Buffer();
double* loadavg = static_cast<double*>(ab->GetContents().Data());
double* loadavg = static_cast<double*>(ab->GetBackingStore()->Data());
uv_loadavg(loadavg);
}

Expand Down
10 changes: 5 additions & 5 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ static void CPUUsage(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 2);
Local<ArrayBuffer> ab = array->Buffer();
double* fields = static_cast<double*>(ab->GetContents().Data());
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());

// Set the Float64Array elements to be user / system values in microseconds.
fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
Expand Down Expand Up @@ -148,7 +148,7 @@ static void Hrtime(const FunctionCallbackInfo<Value>& args) {
uint64_t t = uv_hrtime();

Local<ArrayBuffer> ab = args[0].As<Uint32Array>()->Buffer();
uint32_t* fields = static_cast<uint32_t*>(ab->GetContents().Data());
uint32_t* fields = static_cast<uint32_t*>(ab->GetBackingStore()->Data());

fields[0] = (t / NANOS_PER_SEC) >> 32;
fields[1] = (t / NANOS_PER_SEC) & 0xffffffff;
Expand All @@ -157,7 +157,7 @@ static void Hrtime(const FunctionCallbackInfo<Value>& args) {

static void HrtimeBigInt(const FunctionCallbackInfo<Value>& args) {
Local<ArrayBuffer> ab = args[0].As<BigUint64Array>()->Buffer();
uint64_t* fields = static_cast<uint64_t*>(ab->GetContents().Data());
uint64_t* fields = static_cast<uint64_t*>(ab->GetBackingStore()->Data());
fields[0] = uv_hrtime();
}

Expand Down Expand Up @@ -204,7 +204,7 @@ static void MemoryUsage(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 4);
Local<ArrayBuffer> ab = array->Buffer();
double* fields = static_cast<double*>(ab->GetContents().Data());
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());

fields[0] = rss;
fields[1] = v8_heap_stats.total_heap_size();
Expand Down Expand Up @@ -301,7 +301,7 @@ static void ResourceUsage(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 16);
Local<ArrayBuffer> ab = array->Buffer();
double* fields = static_cast<double*>(ab->GetContents().Data());
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());

fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
fields[1] = MICROS_PER_SEC * rusage.ru_stime.tv_sec + rusage.ru_stime.tv_usec;
Expand Down
4 changes: 3 additions & 1 deletion src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,9 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo<Value>& args) {

Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, sizeof(resource_limits_));
memcpy(ab->GetContents().Data(), resource_limits_, sizeof(resource_limits_));
memcpy(ab->GetBackingStore()->Data(),
resource_limits_,
sizeof(resource_limits_));
return Float64Array::New(ab, 0, kTotalResourceLimitCount);
}

Expand Down
3 changes: 2 additions & 1 deletion src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ class ZlibStream : public CompressionStream<ZlibContext> {
CHECK(args[4]->IsUint32Array());
Local<Uint32Array> array = args[4].As<Uint32Array>();
Local<ArrayBuffer> ab = array->Buffer();
uint32_t* write_result = static_cast<uint32_t*>(ab->GetContents().Data());
uint32_t* write_result = static_cast<uint32_t*>(
ab->GetBackingStore()->Data());

CHECK(args[5]->IsFunction());
Local<Function> write_js_callback = args[5].As<Function>();
Expand Down
2 changes: 1 addition & 1 deletion src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
length_ = abv->ByteLength();
if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
data_ = static_cast<T*>(abv->Buffer()->GetContents().Data()) +
data_ = static_cast<T*>(abv->Buffer()->GetBackingStore()->Data()) +
abv->ByteOffset();
} else {
abv->CopyContents(stack_storage_, sizeof(stack_storage_));
Expand Down
5 changes: 3 additions & 2 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,12 @@ class BufferValue : public MaybeStackBuffer<char> {
#define SPREAD_BUFFER_ARG(val, name) \
CHECK((val)->IsArrayBufferView()); \
v8::Local<v8::ArrayBufferView> name = (val).As<v8::ArrayBufferView>(); \
v8::ArrayBuffer::Contents name##_c = name->Buffer()->GetContents(); \
std::shared_ptr<v8::BackingStore> name##_bs = \
name->Buffer()->GetBackingStore(); \
const size_t name##_offset = name->ByteOffset(); \
const size_t name##_length = name->ByteLength(); \
char* const name##_data = \
static_cast<char*>(name##_c.Data()) + name##_offset; \
static_cast<char*>(name##_bs->Data()) + name##_offset; \
if (name##_length > 0) \
CHECK_NE(name##_data, nullptr);

Expand Down
4 changes: 2 additions & 2 deletions test/addons/openssl-binding/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ inline void RandomBytes(const v8::FunctionCallbackInfo<v8::Value>& info) {
auto byte_length = view->ByteLength();
assert(view->HasBuffer());
auto buffer = view->Buffer();
auto contents = buffer->GetContents();
auto data = static_cast<unsigned char*>(contents.Data()) + byte_offset;
auto contents = buffer->GetBackingStore();
auto data = static_cast<unsigned char*>(contents->Data()) + byte_offset;
assert(RAND_poll());
auto rval = RAND_bytes(data, static_cast<int>(byte_length));
info.GetReturnValue().Set(rval > 0);
Expand Down
4 changes: 2 additions & 2 deletions test/addons/zlib-binding/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ inline void CompressBytes(const v8::FunctionCallbackInfo<v8::Value>& info) {
auto byte_length = view->ByteLength();
assert(view->HasBuffer());
auto buffer = view->Buffer();
auto contents = buffer->GetContents();
auto data = static_cast<unsigned char*>(contents.Data()) + byte_offset;
auto contents = buffer->GetBackingStore();
auto data = static_cast<unsigned char*>(contents->Data()) + byte_offset;

Bytef buf[1024];

Expand Down

0 comments on commit 1a92c88

Please sign in to comment.