Skip to content
This repository was archived by the owner on Dec 4, 2023. It is now read-only.

Commit 5ca52d5

Browse files
committed
remove backref code.
The current backref implementation is designed to ensure that if a V8 object is converted to a ruby object, it gets the same `Data_Wrap_Struct` every time it is passed to Ruby. While this is not necessarily a bad approach, it is definitely what I'd classify as an optimization. The current issues with The Ruby Racer all center around stability and none center around performance, and so I'd like to remove it until such time as we have that as a hard requirement. I know that it will result in more object allocations in both the Ruby world and the V8 world, but the trade off is code that is more difficult to reason about, and might be prone to subtle race conditions that involve multiple garbage collectors. The Ruby side already has an identity map, so that we can resolve 'sameness' at the layer above.
1 parent 9ab2d8b commit 5ca52d5

9 files changed

Lines changed: 8 additions & 133 deletions

File tree

ext/v8/backref.cc

Lines changed: 0 additions & 47 deletions
This file was deleted.

ext/v8/backref.h

Lines changed: 0 additions & 29 deletions
This file was deleted.

ext/v8/init.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ extern "C" {
1212
Isolate::Init();
1313
Handles::Init();
1414
Context::Init();
15-
Backref::Init();
1615
Maybe::Init();
1716
Value::Init();
1817
Object::Init();
@@ -35,7 +34,6 @@ extern "C" {
3534
// Signature::Init();
3635
// Date::Init();
3736
// Constants::Init();
38-
// External::Init();
3937
// Template::Init();
4038
// Stack::Init();
4139
// Message::Init();

ext/v8/object.cc

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -55,46 +55,14 @@ namespace rr {
5555
}
5656

5757
Object::operator VALUE() {
58-
Locker lock(getIsolate());
58+
Isolate isolate(getIsolate());
59+
Locker lock(isolate);
5960

60-
if (handle.IsEmpty()) {
61-
return Qnil;
61+
if (handle->IsFunction()) {
62+
return Function(isolate, handle.As<v8::Function>());
6263
}
6364

64-
Backref* backref;
65-
66-
v8::Local<v8::String> key(v8::String::NewFromUtf8(getIsolate(), "rr::Backref"));
67-
v8::Local<v8::Value> external = handle->GetHiddenValue(key);
68-
69-
VALUE value;
70-
71-
if (external.IsEmpty()) {
72-
value = downcast();
73-
backref = new Backref(value);
74-
75-
handle->SetHiddenValue(key, backref->toExternal(getIsolate()));
76-
} else {
77-
v8::External* wrapper = v8::External::Cast(*external);
78-
backref = (Backref*)wrapper->Value();
79-
value = backref->get();
80-
81-
if (!RTEST(value)) {
82-
value = downcast();
83-
backref->set(value);
84-
}
85-
}
86-
87-
return value;
88-
}
89-
90-
VALUE Object::downcast() {
91-
Locker lock(getIsolate());
92-
9365
// TODO: Enable this when the methods are implemented
94-
// if (handle->IsFunction()) {
95-
// return Function((v8::Handle<v8::Function>) v8::Function::Cast(*handle));
96-
// }
97-
//
9866
// if (handle->IsArray()) {
9967
// return Array((v8::Handle<v8::Array>)v8::Array::Cast(*handle));
10068
// }

ext/v8/object.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,7 @@ namespace rr {
5656

5757
inline Object(VALUE value) : Ref<v8::Object>(value) {}
5858
inline Object(v8::Isolate* isolate, v8::Handle<v8::Object> object) : Ref<v8::Object>(isolate, object) {}
59-
60-
virtual operator VALUE();
61-
62-
protected:
63-
VALUE downcast();
59+
operator VALUE();
6460
};
6561

6662
}

ext/v8/rr.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ inline VALUE not_implemented(const char* message) {
3333
#include "context.h"
3434

3535
#include "value.h"
36-
#include "backref.h"
37-
3836
#include "object.h"
3937
#include "array.h"
4038
#include "primitive.h"

spec/c/function_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424

2525
fn.Call(@ctx.Global, [one, two, 3])
2626

27-
expect(@ctx.Global.Get(@ctx, V8::C::String.NewFromUtf8(@isolate, 'one')).FromJust()).to eq one
28-
expect(@ctx.Global.Get(@ctx, V8::C::String.NewFromUtf8(@isolate, 'two')).FromJust()).to eq two
27+
expect(@ctx.Global.Get(@ctx, V8::C::String.NewFromUtf8(@isolate, 'one')).FromJust().StrictEquals(one)).to be true
28+
expect(@ctx.Global.Get(@ctx, V8::C::String.NewFromUtf8(@isolate, 'two')).FromJust().StrictEquals(two)).to be true
2929
expect(@ctx.Global.Get(@ctx, V8::C::String.NewFromUtf8(@isolate, 'three')).FromJust().Value()).to eq 3
3030
end
3131

spec/c/object_spec.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,4 @@
5151
# o.Set(property, V8::C::String::New('Bro! '))
5252
# o.Get(property).Utf8Value().should eql 'Bro! I am Legend'
5353
# end
54-
55-
it 'always returns the same ruby object for the same V8 object' do
56-
key = V8::C::String.NewFromUtf8(@isolate, 'two')
57-
one = V8::C::Object.New(@isolate)
58-
two = V8::C::Object.New(@isolate)
59-
60-
one.Set(@ctx, key, two)
61-
expect(one.Get(@ctx, key).FromJust()).to be two
62-
end
6354
end

spec/c/value_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ def convert(value)
3535
object = V8::C::Object.New(@isolate)
3636
object.Set(@ctx, V8::C::String.NewFromUtf8(@isolate, 'test'), 1)
3737

38-
expect(convert(object)).to eq object
38+
expect(convert(object).StrictEquals(object)).to be true
3939
end
4040
end

0 commit comments

Comments
 (0)