-
Notifications
You must be signed in to change notification settings - Fork 121
Update upstream V8 to 6.0 #243
Comments
cc @SamSaffron |
❤️ love that you are keeping this up to date! |
I just know you like to run benchmarks, so I'm giving you an excuse to ;) Will release beta as soon as I see the CI green. |
awesome, we are going to want to test this quick smart given: https://twitter.com/bmeurer/status/890247544764289025 v8 6.0 was released yesterday 🎊 |
Interesting. I wish it was easier to benchmark the 'real world' code in my game without pushing it live. I wonder if waiting until 6.1 is released is the right bet. 5.7 has been treating me really well wrt stability (much more important than performance & features in my use case) |
I think 6.0 should in theory help, will bench it and see. I do not expect any stability issues though, just minor perf issues. |
haha, in this project, if 'tie-scenario' changes in the underlying sort implementation (apparently not specified in js standard) or something like the random number impl; it can and will break so many things. anyhow, over-details. I'll probably take a stab at 6.0 and rollback if necessary. :) |
Beta is up. |
need to fix #244 before release though |
not sure if this is a v8 issue or miniracer issue, but I'm getting this on a
|
confirmed @ignisf appears I am having trouble linking this with miniracer /home/sam/Source/mini_racer/lib/mini_racer.rb:2:in `require': /home/sam/Source/mini_racer/lib/mini_racer_extension.so: undefined symbol: _ZTIN2v811ArrayBuffer9AllocatorE - /home/sam/Source/mini_racer/lib/mini_racer_extension.so (LoadError) Maybe arraybufferallocator got removed or something? |
confirmed on linux, too. Looking into it. |
I screwed up with the patches... |
anything we can do to help with this stuff? |
Hya, I've been exceptionally busy lately. Unfortunately it's not the patches. I've been hitting this issue even with them removed D:. Still investigating... Things left to do:
|
yikes! That's weird.. |
No luck with the newer version... And without any params the build fails even before it has started... D: |
@seanmakesgames @SamSaffron, I'm beginning to think it's a breaking change in upstream V8 causing this. Not sure what yet. Working:
Not working:
Tried to make the linker tell me what's wrong by making it produce a static library:
|
6.1: /**
* An instance of the built-in ArrayBuffer constructor (ES6 draft 15.13.5).
*/
class V8_EXPORT ArrayBuffer : public Object {
public:
/**
* A thread-safe allocator that V8 uses to allocate |ArrayBuffer|'s memory.
* The allocator is a global V8 setting. It has to be set via
* Isolate::CreateParams.
*
* Memory allocated through this allocator by V8 is accounted for as external
* memory by V8. Note that V8 keeps track of the memory for all internalized
* |ArrayBuffer|s. Responsibility for tracking external memory (using
* Isolate::AdjustAmountOfExternalAllocatedMemory) is handed over to the
* embedder upon externalization and taken over upon internalization (creating
* an internalized buffer from an existing buffer).
*
* Note that it is unsafe to call back into V8 from any of the allocator
* functions.
*/
class V8_EXPORT Allocator { // NOLINT
public:
virtual ~Allocator() {}
/**
* Allocate |length| bytes. Return NULL if allocation is not successful.
* Memory should be initialized to zeroes.
*/
virtual void* Allocate(size_t length) = 0;
/**
* Allocate |length| bytes. Return NULL if allocation is not successful.
* Memory does not have to be initialized.
*/
virtual void* AllocateUninitialized(size_t length) = 0;
/**
* Reserved |length| bytes, but do not commit the memory. Must call
* |SetProtection| to make memory accessible.
*/
// TODO(eholk): make this pure virtual once blink implements this.
virtual void* Reserve(size_t length);
/**
* Free the memory block of size |length|, pointed to by |data|.
* That memory is guaranteed to be previously allocated by |Allocate|.
*/
virtual void Free(void* data, size_t length) = 0;
enum class AllocationMode { kNormal, kReservation };
/**
* Free the memory block of size |length|, pointed to by |data|.
* That memory is guaranteed to be previously allocated by |Allocate| or
* |Reserve|, depending on |mode|.
*/
// TODO(eholk): make this pure virtual once blink implements this.
virtual void Free(void* data, size_t length, AllocationMode mode);
enum class Protection { kNoAccess, kReadWrite };
/**
* Change the protection on a region of memory.
*
* On platforms that make a distinction between reserving and committing
* memory, changing the protection to kReadWrite must also ensure the memory
* is committed.
*/
// TODO(eholk): make this pure virtual once blink implements this.
virtual void SetProtection(void* data, size_t length,
Protection protection);
/**
* malloc/free based convenience allocator.
*
* Caller takes ownership, i.e. the returned object needs to be freed using
* |delete allocator| once it is no longer in use.
*/
static Allocator* NewDefaultAllocator();
}; 5.9: /**
* An instance of the built-in ArrayBuffer constructor (ES6 draft 15.13.5).
*/
class V8_EXPORT ArrayBuffer : public Object {
public:
/**
* A thread-safe allocator that V8 uses to allocate |ArrayBuffer|'s memory.
* The allocator is a global V8 setting. It has to be set via
* Isolate::CreateParams.
*
* Memory allocated through this allocator by V8 is accounted for as external
* memory by V8. Note that V8 keeps track of the memory for all internalized
* |ArrayBuffer|s. Responsibility for tracking external memory (using
* Isolate::AdjustAmountOfExternalAllocatedMemory) is handed over to the
* embedder upon externalization and taken over upon internalization (creating
* an internalized buffer from an existing buffer).
*
* Note that it is unsafe to call back into V8 from any of the allocator
* functions.
*/
class V8_EXPORT Allocator { // NOLINT
public:
virtual ~Allocator() {}
/**
* Allocate |length| bytes. Return NULL if allocation is not successful.
* Memory should be initialized to zeroes.
*/
virtual void* Allocate(size_t length) = 0;
/**
* Allocate |length| bytes. Return NULL if allocation is not successful.
* Memory does not have to be initialized.
*/
virtual void* AllocateUninitialized(size_t length) = 0;
/**
* Free the memory block of size |length|, pointed to by |data|.
* That memory is guaranteed to be previously allocated by |Allocate|.
*/
virtual void Free(void* data, size_t length) = 0;
/**
* malloc/free based convenience allocator.
*
* Caller takes ownership, i.e. the returned object needs to be freed using
* |delete allocator| once it is no longer in use.
*/
static Allocator* NewDefaultAllocator();
}; |
Hmmm... This seems to fix it... diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc
index 63c75d4..6d6f43c 100644
--- a/ext/mini_racer_extension/mini_racer_extension.cc
+++ b/ext/mini_racer_extension/mini_racer_extension.cc
@@ -11,20 +11,6 @@
using namespace v8;
-class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
- public:
- virtual void* Allocate(size_t length) {
- void* data = AllocateUninitialized(length);
- return data == NULL ? data : memset(data, 0, length);
- }
- virtual void* AllocateUninitialized(size_t length) {
- return malloc(length);
- }
- virtual void Free(void* data, size_t) {
- free(data);
- }
-};
-
typedef struct {
const char* data;
int raw_size;
@@ -32,7 +18,7 @@ typedef struct {
typedef struct {
Isolate* isolate;
- ArrayBufferAllocator* allocator;
+ v8::ArrayBuffer::Allocator* allocator;
StartupData* startup_data;
bool interrupted;
bool disposed;
@@ -453,7 +439,7 @@ static VALUE rb_isolate_init_with_snapshot(VALUE self, VALUE snapshot) {
init_v8();
- isolate_info->allocator = new ArrayBufferAllocator();
+ isolate_info->allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
isolate_info->interrupted = false;
isolate_info->refs_count = 1; |
Nice, I support that change!!!
…On Tue, Aug 22, 2017 at 9:33 AM, Petko Bordjukov ***@***.***> wrote:
Hmmm...
This seems to fix it...
diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc
index 63c75d4..6d6f43c 100644--- a/ext/mini_racer_extension/mini_racer_extension.cc+++ b/ext/mini_racer_extension/mini_racer_extension.cc@@ -11,20 +11,6 @@
using namespace v8;
-class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {- public:- virtual void* Allocate(size_t length) {- void* data = AllocateUninitialized(length);- return data == NULL ? data : memset(data, 0, length);- }- virtual void* AllocateUninitialized(size_t length) {- return malloc(length);- }- virtual void Free(void* data, size_t) {- free(data);- }-};-
typedef struct {
const char* data;
int raw_size;@@ -32,7 +18,7 @@ typedef struct {
typedef struct {
Isolate* isolate;- ArrayBufferAllocator* allocator;+ v8::ArrayBuffer::Allocator* allocator;
StartupData* startup_data;
bool interrupted;
bool disposed;@@ -453,7 +439,7 @@ static VALUE rb_isolate_init_with_snapshot(VALUE self, VALUE snapshot) {
init_v8();
- isolate_info->allocator = new ArrayBufferAllocator();+ isolate_info->allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
isolate_info->interrupted = false;
isolate_info->refs_count = 1;
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#243 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAUXeFr2h8vYGVh_VCvKy3xxAKQ-tAIks5satiZgaJpZM4Oj_1k>
.
|
Opened rubyjs/mini_racer#64 |
mini_racer now works with the 6.0 beta |
v6.0.286.54.0beta1 is up |
v6.1.534.22.0beta1 will soon be up, too |
did you guys ever figure out what changed that caused this problem in the first place? I'm running into the v8::ArrayBufferAllocator typeinfo problem myself.. though I'll probably swap to the convenience allocator, too |
@xaxxon, hey Yes -- as far as I understood, the underlying V8 interface for the Allocator changed and we had unimplemented virtual methods as a result of this. (I love how I needed to debug a cryptic linking error to get to this). Just changing to the default implementation allows us to not worry about them changing the interface again, hopefully. |
When that happens usually clang gives an error saying that it's likely the
first virtual function isn't implemented but I didn't see that. Weird.
I didn't realize he interface had changed but undid the same as you anyhow.
Thanks for the answer.
…On Tue, Sep 5, 2017 at 5:31 PM Petko Bordjukov ***@***.***> wrote:
@xaxxon <https://github.com/xaxxon>, hey
Yes -- as far as I understood, the underlying V8 interface for the
Allocator changed and we had unimplemented virtual methods as a result of
this. (I love how I needed to debug a cryptic linking error to get to this).
Just changing to the default implementation allows us to not worry about
them changing the interface again, hopefully.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#243 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAIychfaaxG9rd0w2GYqBkJ2B6MXK6pIks5sfefMgaJpZM4Oj_1k>
.
|
No description provided.
The text was updated successfully, but these errors were encountered: