Skip to content

Commit 6918512

Browse files
byrootbyroot
authored
Merge pull request #337 from Shopify/write-barriers
Implement Write Barrier for MessagePack::Factory
2 parents 78b48c6 + 03bc9a9 commit 6918512

7 files changed

Lines changed: 50 additions & 35 deletions

File tree

‎ext/msgpack/factory_class.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ static const rb_data_type_t factory_data_type = {
7575
.dfree = Factory_free,
7676
.dsize = Factory_memsize,
7777
},
78-
.flags = RUBY_TYPED_FREE_IMMEDIATELY
78+
.flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED
7979
};
8080

8181
static inline msgpack_factory_t *Factory_get(VALUE object)
@@ -98,7 +98,7 @@ static VALUE Factory_initialize(int argc, VALUE* argv, VALUE self)
9898
{
9999
msgpack_factory_t *fc = Factory_get(self);
100100

101-
msgpack_packer_ext_registry_init(&fc->pkrg);
101+
msgpack_packer_ext_registry_init(self, &fc->pkrg);
102102
// fc->ukrg is lazily initialized
103103

104104
fc->has_symbol_ext_type = false;
@@ -124,7 +124,7 @@ static VALUE Factory_dup(VALUE self)
124124
cloned_fc->has_symbol_ext_type = fc->has_symbol_ext_type;
125125
cloned_fc->pkrg = fc->pkrg;
126126
msgpack_unpacker_ext_registry_borrow(fc->ukrg, &cloned_fc->ukrg);
127-
msgpack_packer_ext_registry_dup(&fc->pkrg, &cloned_fc->pkrg);
127+
msgpack_packer_ext_registry_dup(clone, &fc->pkrg, &cloned_fc->pkrg);
128128

129129
return clone;
130130
}
@@ -139,7 +139,7 @@ static VALUE Factory_freeze(VALUE self) {
139139
// If the factory is frozen, we can safely share the packer cache between
140140
// all packers. So we eagerly create it now so it's available when #packer
141141
// is called.
142-
fc->pkrg.cache = rb_hash_new();
142+
RB_OBJ_WRITE(self, &fc->pkrg.cache, rb_hash_new());
143143
}
144144
}
145145

@@ -158,7 +158,7 @@ VALUE MessagePack_Factory_packer(int argc, VALUE* argv, VALUE self)
158158

159159
msgpack_packer_t* pk = MessagePack_Packer_get(packer);
160160
msgpack_packer_ext_registry_destroy(&pk->ext_registry);
161-
msgpack_packer_ext_registry_dup(&fc->pkrg, &pk->ext_registry);
161+
msgpack_packer_ext_registry_borrow(packer, &fc->pkrg, &pk->ext_registry);
162162
pk->has_bigint_ext_type = fc->has_bigint_ext_type;
163163
pk->has_symbol_ext_type = fc->has_symbol_ext_type;
164164

@@ -289,8 +289,8 @@ static VALUE Factory_register_type(int argc, VALUE* argv, VALUE self)
289289
}
290290
}
291291

292-
msgpack_packer_ext_registry_put(&fc->pkrg, ext_module, ext_type, flags, packer_proc, packer_arg);
293-
msgpack_unpacker_ext_registry_put(&fc->ukrg, ext_module, ext_type, flags, unpacker_proc, unpacker_arg);
292+
msgpack_packer_ext_registry_put(self, &fc->pkrg, ext_module, ext_type, flags, packer_proc, packer_arg);
293+
msgpack_unpacker_ext_registry_put(self, &fc->ukrg, ext_module, ext_type, flags, unpacker_proc, unpacker_arg);
294294

295295
return Qnil;
296296
}

‎ext/msgpack/packer_class.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ VALUE MessagePack_Packer_initialize(int argc, VALUE* argv, VALUE self)
106106

107107
msgpack_packer_t *pk = MessagePack_Packer_get(self);
108108

109-
msgpack_packer_ext_registry_init(&pk->ext_registry);
110-
pk->buffer_ref = Qnil;
109+
msgpack_packer_ext_registry_init(self, &pk->ext_registry);
110+
pk->buffer_ref = MessagePack_Buffer_wrap(PACKER_BUFFER_(pk), self);
111111

112112
MessagePack_Buffer_set_options(PACKER_BUFFER_(pk), io, options);
113113

@@ -391,7 +391,7 @@ static VALUE Packer_register_type(int argc, VALUE* argv, VALUE self)
391391
rb_raise(rb_eArgError, "expected Module/Class but found %s.", rb_obj_classname(ext_module));
392392
}
393393

394-
msgpack_packer_ext_registry_put(&pk->ext_registry, ext_module, ext_type, 0, proc, arg);
394+
msgpack_packer_ext_registry_put(self, &pk->ext_registry, ext_module, ext_type, 0, proc, arg);
395395

396396
if (ext_module == rb_cSymbol) {
397397
pk->has_symbol_ext_type = true;

‎ext/msgpack/packer_ext_registry.c‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ void msgpack_packer_ext_registry_static_init(void)
2828
void msgpack_packer_ext_registry_static_destroy(void)
2929
{ }
3030

31-
void msgpack_packer_ext_registry_init(msgpack_packer_ext_registry_t* pkrg)
31+
void msgpack_packer_ext_registry_init(VALUE owner, msgpack_packer_ext_registry_t* pkrg)
3232
{
33-
pkrg->hash = Qnil;
34-
pkrg->cache = Qnil;
33+
RB_OBJ_WRITE(owner, &pkrg->hash, Qnil);
34+
RB_OBJ_WRITE(owner, &pkrg->cache, Qnil);
3535
}
3636

3737
void msgpack_packer_ext_registry_mark(msgpack_packer_ext_registry_t* pkrg)
@@ -40,32 +40,46 @@ void msgpack_packer_ext_registry_mark(msgpack_packer_ext_registry_t* pkrg)
4040
rb_gc_mark(pkrg->cache);
4141
}
4242

43-
void msgpack_packer_ext_registry_dup(msgpack_packer_ext_registry_t* src,
43+
void msgpack_packer_ext_registry_borrow(VALUE owner, msgpack_packer_ext_registry_t* src,
4444
msgpack_packer_ext_registry_t* dst)
4545
{
46-
if(RTEST(src->hash) && !rb_obj_frozen_p(src->hash)) {
47-
dst->hash = rb_hash_dup(src->hash);
48-
dst->cache = RTEST(src->cache) ? rb_hash_dup(src->cache) : Qnil;
46+
if(RTEST(src->hash)) {
47+
if(rb_obj_frozen_p(src->hash)) {
48+
// If the type registry is frozen we can safely share it, and share the cache as well.
49+
RB_OBJ_WRITE(owner, &dst->hash, src->hash);
50+
RB_OBJ_WRITE(owner, &dst->cache, src->cache);
51+
} else {
52+
RB_OBJ_WRITE(owner, &dst->hash, rb_hash_dup(src->hash));
53+
RB_OBJ_WRITE(owner, &dst->cache, NIL_P(src->cache) ? Qnil : rb_hash_dup(src->cache));
54+
}
4955
} else {
50-
// If the type registry is frozen we can safely share it, and share the cache as well.
51-
dst->hash = src->hash;
52-
dst->cache = src->cache;
56+
RB_OBJ_WRITE(owner, &dst->hash, Qnil);
57+
RB_OBJ_WRITE(owner, &dst->cache, Qnil);
5358
}
5459
}
5560

56-
VALUE msgpack_packer_ext_registry_put(msgpack_packer_ext_registry_t* pkrg,
61+
void msgpack_packer_ext_registry_dup(VALUE owner, msgpack_packer_ext_registry_t* src,
62+
msgpack_packer_ext_registry_t* dst)
63+
{
64+
RB_OBJ_WRITE(owner, &dst->hash, NIL_P(src->hash) ? Qnil : rb_hash_dup(src->hash));
65+
RB_OBJ_WRITE(owner, &dst->cache, NIL_P(src->cache) ? Qnil : rb_hash_dup(src->cache));
66+
}
67+
68+
void msgpack_packer_ext_registry_put(VALUE owner, msgpack_packer_ext_registry_t* pkrg,
5769
VALUE ext_module, int ext_type, int flags, VALUE proc, VALUE arg)
5870
{
59-
if (!RTEST(pkrg->hash)) {
60-
pkrg->hash = rb_hash_new();
71+
if(NIL_P(pkrg->hash)) {
72+
RB_OBJ_WRITE(owner, &pkrg->hash, rb_hash_new());
6173
}
6274

63-
if (RTEST(pkrg->cache)) {
75+
if(NIL_P(pkrg->cache)) {
76+
RB_OBJ_WRITE(owner, &pkrg->cache, rb_hash_new());
77+
} else {
6478
/* clear lookup cache not to miss added type */
6579
rb_hash_clear(pkrg->cache);
6680
}
6781

6882
// TODO: Ruby embeded array limit is 3, merging `proc` and `arg` would be good.
6983
VALUE entry = rb_ary_new3(4, INT2FIX(ext_type), proc, arg, INT2FIX(flags));
70-
return rb_hash_aset(pkrg->hash, ext_module, entry);
84+
rb_hash_aset(pkrg->hash, ext_module, entry);
7185
}

‎ext/msgpack/packer_ext_registry.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,20 @@ void msgpack_packer_ext_registry_static_init(void);
3535

3636
void msgpack_packer_ext_registry_static_destroy(void);
3737

38-
void msgpack_packer_ext_registry_init(msgpack_packer_ext_registry_t* pkrg);
38+
void msgpack_packer_ext_registry_init(VALUE owner, msgpack_packer_ext_registry_t* pkrg);
3939

4040
static inline void msgpack_packer_ext_registry_destroy(msgpack_packer_ext_registry_t* pkrg)
4141
{ }
4242

4343
void msgpack_packer_ext_registry_mark(msgpack_packer_ext_registry_t* pkrg);
4444

45-
void msgpack_packer_ext_registry_dup(msgpack_packer_ext_registry_t* src,
45+
void msgpack_packer_ext_registry_borrow(VALUE owner, msgpack_packer_ext_registry_t* src,
4646
msgpack_packer_ext_registry_t* dst);
4747

48-
VALUE msgpack_packer_ext_registry_put(msgpack_packer_ext_registry_t* pkrg,
48+
void msgpack_packer_ext_registry_dup(VALUE owner, msgpack_packer_ext_registry_t* src,
49+
msgpack_packer_ext_registry_t* dst);
50+
51+
void msgpack_packer_ext_registry_put(VALUE owner, msgpack_packer_ext_registry_t* pkrg,
4952
VALUE ext_module, int ext_type, int flags, VALUE proc, VALUE arg);
5053

5154
static int msgpack_packer_ext_find_superclass(VALUE key, VALUE value, VALUE arg)
@@ -129,9 +132,6 @@ static inline VALUE msgpack_packer_ext_registry_lookup(msgpack_packer_ext_regist
129132
VALUE superclass = args[1];
130133
if(superclass != Qnil) {
131134
VALUE superclass_type = rb_hash_lookup(pkrg->hash, superclass);
132-
if (!RTEST(pkrg->cache)) {
133-
pkrg->cache = rb_hash_new();
134-
}
135135
rb_hash_aset(pkrg->cache, lookup_class, superclass_type);
136136
*ext_type_result = FIX2INT(rb_ary_entry(superclass_type, 0));
137137
*ext_flags_result = FIX2INT(rb_ary_entry(superclass_type, 3));

‎ext/msgpack/unpacker_class.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static VALUE Unpacker_register_type(int argc, VALUE* argv, VALUE self)
382382
rb_raise(rb_eRangeError, "integer %d too big to convert to `signed char'", ext_type);
383383
}
384384

385-
msgpack_unpacker_ext_registry_put(&uk->ext_registry, ext_module, ext_type, 0, proc, arg);
385+
msgpack_unpacker_ext_registry_put(self, &uk->ext_registry, ext_module, ext_type, 0, proc, arg);
386386

387387
return Qnil;
388388
}

‎ext/msgpack/unpacker_ext_registry.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@ void msgpack_unpacker_ext_registry_release(msgpack_unpacker_ext_registry_t* ukrg
7676
}
7777
}
7878

79-
void msgpack_unpacker_ext_registry_put(msgpack_unpacker_ext_registry_t** ukrg,
79+
void msgpack_unpacker_ext_registry_put(VALUE owner, msgpack_unpacker_ext_registry_t** ukrg,
8080
VALUE ext_module, int ext_type, int flags, VALUE proc, VALUE arg)
8181
{
8282
msgpack_unpacker_ext_registry_t* ext_registry = msgpack_unpacker_ext_registry_cow(*ukrg);
8383

84-
ext_registry->array[ext_type + 128] = rb_ary_new3(4, ext_module, proc, arg, INT2FIX(flags));
84+
VALUE entry = rb_ary_new3(4, ext_module, proc, arg, INT2FIX(flags));
85+
RB_OBJ_WRITE(owner, &ext_registry->array[ext_type + 128], entry);
8586
*ukrg = ext_registry;
8687
}

‎ext/msgpack/unpacker_ext_registry.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ static inline void msgpack_unpacker_ext_registry_borrow(msgpack_unpacker_ext_reg
4747

4848
void msgpack_unpacker_ext_registry_mark(msgpack_unpacker_ext_registry_t* ukrg);
4949

50-
void msgpack_unpacker_ext_registry_put(msgpack_unpacker_ext_registry_t** ukrg,
50+
void msgpack_unpacker_ext_registry_put(VALUE owner, msgpack_unpacker_ext_registry_t** ukrg,
5151
VALUE ext_module, int ext_type, int flags, VALUE proc, VALUE arg);
5252

5353
static inline VALUE msgpack_unpacker_ext_registry_lookup(msgpack_unpacker_ext_registry_t* ukrg,

0 commit comments

Comments
 (0)