Skip to content

Commit cc84bf3

Browse files
byrootbyroot
committed
Optimize Packer and Unpacker initialize methods
Particularly through the common `MessagePack.load / dump`. First we accept `nil` in place of the option hash, which allows to skip several hash lookup when no option is passed. Then if we actually pass an option hash, we used static symbols instead of goign through the ID2SYM every time. Finally for MessagePack.pack, we accept `nil` in place of var args, as to avoid a costly `*rest` forwarding.
1 parent 84ff463 commit cc84bf3

8 files changed

Lines changed: 100 additions & 55 deletions

File tree

‎doclib/msgpack/factory.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def packer(*args)
3131
#
3232
# See Packer#initialize for supported options.
3333
#
34-
def dump(obj, options={})
34+
def dump(obj, options=nil)
3535
end
3636
alias pack dump
3737

@@ -57,7 +57,7 @@ def unpacker(*args)
5757
#
5858
# See Unpacker#initialize for supported options.
5959
#
60-
def load(data, options={})
60+
def load(data, options=nil)
6161
end
6262
alias unpack load
6363

‎ext/java/org/msgpack/jruby/Factory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public IRubyObject initialize(ThreadContext ctx) {
5050
return this;
5151
}
5252

53-
@JRubyMethod(name = "packer", optional = 1)
53+
@JRubyMethod(name = "packer", optional = 2)
5454
public Packer packer(ThreadContext ctx, IRubyObject[] args) {
5555
return Packer.newPacker(ctx, extensionRegistry(), hasSymbolExtType, args);
5656
}

‎ext/java/org/msgpack/jruby/Packer.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,18 @@ public IRubyObject allocate(Ruby runtime, RubyClass type) {
5050
public IRubyObject initialize(ThreadContext ctx, IRubyObject[] args) {
5151
boolean compatibilityMode = false;
5252
Ruby runtime = ctx.runtime;
53-
if (args.length > 0 && args[args.length - 1] instanceof RubyHash) {
54-
RubyHash options = (RubyHash) args[args.length - 1];
55-
IRubyObject mode = options.fastARef(runtime.newSymbol("compatibility_mode"));
56-
compatibilityMode = (mode != null) && mode.isTrue();
53+
if (args.length > 0) {
54+
RubyHash options = null;
55+
if (args[args.length - 1] instanceof RubyHash) {
56+
options = (RubyHash) args[args.length - 1];
57+
} else if (args.length > 1 && args[args.length - 2] instanceof RubyHash) {
58+
options = (RubyHash) args[args.length - 2];
59+
}
60+
61+
if (options != null) {
62+
IRubyObject mode = options.fastARef(runtime.newSymbol("compatibility_mode"));
63+
compatibilityMode = (mode != null) && mode.isTrue();
64+
}
5765
}
5866
if (registry == null) {
5967
// registry is null when allocate -> initialize

‎ext/java/org/msgpack/jruby/Unpacker.java‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,48 @@ public IRubyObject allocate(Ruby runtime, RubyClass klass) {
5656

5757
@JRubyMethod(name = "initialize", optional = 2, visibility = PRIVATE)
5858
public IRubyObject initialize(ThreadContext ctx, IRubyObject[] args) {
59+
Ruby runtime = ctx.runtime;
60+
5961
symbolizeKeys = false;
6062
allowUnknownExt = false;
6163
freeze = false;
62-
if (args.length > 0) {
63-
Ruby runtime = ctx.runtime;
64-
if (args[args.length - 1] instanceof RubyHash) {
65-
RubyHash options = (RubyHash) args[args.length - 1];
66-
IRubyObject sk = options.fastARef(runtime.newSymbol("symbolize_keys"));
67-
if (sk != null) {
68-
symbolizeKeys = sk.isTrue();
69-
}
70-
IRubyObject f = options.fastARef(runtime.newSymbol("freeze"));
71-
if (f != null) {
72-
freeze = f.isTrue();
73-
}
74-
IRubyObject au = options.fastARef(runtime.newSymbol("allow_unknown_ext"));
75-
if (au != null) {
76-
allowUnknownExt = au.isTrue();
77-
}
64+
65+
IRubyObject io = null;
66+
RubyHash options = null;
67+
68+
if (args.length >= 1) {
69+
io = args[0];
70+
}
71+
72+
if (args.length >= 2 && args[1] != runtime.getNil()) {
73+
options = (RubyHash)args[1];
74+
}
75+
76+
if (options == null && io != null && io instanceof RubyHash) {
77+
options = (RubyHash)io;
78+
io = null;
79+
}
80+
81+
if (options != null) {
82+
IRubyObject sk = options.fastARef(runtime.newSymbol("symbolize_keys"));
83+
if (sk != null) {
84+
symbolizeKeys = sk.isTrue();
85+
}
86+
IRubyObject f = options.fastARef(runtime.newSymbol("freeze"));
87+
if (f != null) {
88+
freeze = f.isTrue();
7889
}
79-
if (args[0] != runtime.getNil() && !(args[0] instanceof RubyHash)) {
80-
setStream(ctx, args[0]);
90+
IRubyObject au = options.fastARef(runtime.newSymbol("allow_unknown_ext"));
91+
if (au != null) {
92+
allowUnknownExt = au.isTrue();
8193
}
94+
95+
}
96+
97+
if (io != null && io != runtime.getNil()) {
98+
setStream(ctx, io);
8299
}
100+
83101
return this;
84102
}
85103

‎ext/msgpack/buffer_class.c‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ static ID s_write;
2929
static ID s_append;
3030
static ID s_close;
3131

32+
static VALUE sym_read_reference_threshold;
33+
static VALUE sym_write_reference_threshold;
34+
static VALUE sym_io_buffer_size;
35+
36+
3237
#define BUFFER(from, name) \
3338
msgpack_buffer_t *name = NULL; \
3439
Data_Get_Struct(from, msgpack_buffer_t, name); \
@@ -89,17 +94,17 @@ void MessagePack_Buffer_set_options(msgpack_buffer_t* b, VALUE io, VALUE options
8994
if(options != Qnil) {
9095
VALUE v;
9196

92-
v = rb_hash_aref(options, ID2SYM(rb_intern("read_reference_threshold")));
97+
v = rb_hash_aref(options, sym_read_reference_threshold);
9398
if(v != Qnil) {
9499
msgpack_buffer_set_read_reference_threshold(b, NUM2ULONG(v));
95100
}
96101

97-
v = rb_hash_aref(options, ID2SYM(rb_intern("write_reference_threshold")));
102+
v = rb_hash_aref(options, sym_write_reference_threshold);
98103
if(v != Qnil) {
99104
msgpack_buffer_set_write_reference_threshold(b, NUM2ULONG(v));
100105
}
101106

102-
v = rb_hash_aref(options, ID2SYM(rb_intern("io_buffer_size")));
107+
v = rb_hash_aref(options, sym_io_buffer_size);
103108
if(v != Qnil) {
104109
msgpack_buffer_set_io_buffer_size(b, NUM2ULONG(v));
105110
}
@@ -479,6 +484,10 @@ void MessagePack_Buffer_module_init(VALUE mMessagePack)
479484
s_append = rb_intern("<<");
480485
s_close = rb_intern("close");
481486

487+
sym_read_reference_threshold = ID2SYM(rb_intern("read_reference_threshold"));
488+
sym_write_reference_threshold = ID2SYM(rb_intern("write_reference_threshold"));
489+
sym_io_buffer_size = ID2SYM(rb_intern("io_buffer_size"));
490+
482491
msgpack_buffer_static_init();
483492

484493
cMessagePack_Buffer = rb_define_class_under(mMessagePack, "Buffer", rb_cObject);

‎ext/msgpack/packer_class.c‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ VALUE cMessagePack_Packer;
2828
static ID s_to_msgpack;
2929
static ID s_write;
3030

31+
static VALUE sym_compatibility_mode;
32+
3133
//static VALUE s_packer_value;
3234
//static msgpack_packer_t* s_packer;
3335

@@ -68,29 +70,28 @@ VALUE MessagePack_Packer_alloc(VALUE klass)
6870

6971
VALUE MessagePack_Packer_initialize(int argc, VALUE* argv, VALUE self)
7072
{
73+
if(argc > 2) {
74+
rb_raise(rb_eArgError, "wrong number of arguments (%d for 0..2)", argc);
75+
}
76+
7177
VALUE io = Qnil;
7278
VALUE options = Qnil;
7379

74-
if(argc == 0 || (argc == 1 && argv[0] == Qnil)) {
75-
/* Qnil */
76-
77-
} else if(argc == 1) {
78-
VALUE v = argv[0];
79-
if(rb_type(v) == T_HASH) {
80-
options = v;
81-
} else {
82-
io = v;
83-
}
84-
85-
} else if(argc == 2) {
80+
if(argc >= 1) {
8681
io = argv[0];
82+
}
83+
84+
if(argc == 2) {
8785
options = argv[1];
88-
if(rb_type(options) != T_HASH) {
89-
rb_raise(rb_eArgError, "expected Hash but found %s.", rb_obj_classname(options));
90-
}
86+
}
9187

92-
} else {
93-
rb_raise(rb_eArgError, "wrong number of arguments (%d for 0..2)", argc);
88+
if (options == Qnil && rb_type(io) == T_HASH) {
89+
options = io;
90+
io = Qnil;
91+
}
92+
93+
if(options != Qnil) {
94+
Check_Type(options, T_HASH);
9495
}
9596

9697
PACKER(self, pk);
@@ -103,7 +104,7 @@ VALUE MessagePack_Packer_initialize(int argc, VALUE* argv, VALUE self)
103104
if(options != Qnil) {
104105
VALUE v;
105106

106-
v = rb_hash_aref(options, ID2SYM(rb_intern("compatibility_mode")));
107+
v = rb_hash_aref(options, sym_compatibility_mode);
107108
msgpack_packer_set_compat(pk, RTEST(v));
108109
}
109110

@@ -412,6 +413,8 @@ void MessagePack_Packer_module_init(VALUE mMessagePack)
412413
s_to_msgpack = rb_intern("to_msgpack");
413414
s_write = rb_intern("write");
414415

416+
sym_compatibility_mode = ID2SYM(rb_intern("compatibility_mode"));
417+
415418
msgpack_packer_static_init();
416419
msgpack_packer_ext_registry_static_init();
417420

‎ext/msgpack/unpacker_class.c‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ static VALUE eUnexpectedTypeError;
3333
static VALUE eUnknownExtTypeError;
3434
static VALUE mTypeError; // obsoleted. only for backward compatibility. See #86.
3535

36+
static VALUE sym_symbolize_keys;
37+
static VALUE sym_freeze;
38+
static VALUE sym_allow_unknown_ext;
39+
3640
#define UNPACKER(from, name) \
3741
msgpack_unpacker_t *name = NULL; \
3842
Data_Get_Struct(from, msgpack_unpacker_t, name); \
@@ -83,7 +87,7 @@ VALUE MessagePack_Unpacker_initialize(int argc, VALUE* argv, VALUE self)
8387
} else if(argc == 2) {
8488
io = argv[0];
8589
options = argv[1];
86-
if(rb_type(options) != T_HASH) {
90+
if(options != Qnil && rb_type(options) != T_HASH) {
8791
rb_raise(rb_eArgError, "expected Hash but found %s.", rb_obj_classname(options));
8892
}
8993

@@ -100,13 +104,13 @@ VALUE MessagePack_Unpacker_initialize(int argc, VALUE* argv, VALUE self)
100104
if(options != Qnil) {
101105
VALUE v;
102106

103-
v = rb_hash_aref(options, ID2SYM(rb_intern("symbolize_keys")));
107+
v = rb_hash_aref(options, sym_symbolize_keys);
104108
msgpack_unpacker_set_symbolized_keys(uk, RTEST(v));
105109

106-
v = rb_hash_aref(options, ID2SYM(rb_intern("freeze")));
110+
v = rb_hash_aref(options, sym_freeze);
107111
msgpack_unpacker_set_freeze(uk, RTEST(v));
108112

109-
v = rb_hash_aref(options, ID2SYM(rb_intern("allow_unknown_ext")));
113+
v = rb_hash_aref(options, sym_allow_unknown_ext);
110114
msgpack_unpacker_set_allow_unknown_ext(uk, RTEST(v));
111115
}
112116

@@ -411,6 +415,10 @@ void MessagePack_Unpacker_module_init(VALUE mMessagePack)
411415

412416
eUnknownExtTypeError = rb_define_class_under(mMessagePack, "UnknownExtTypeError", eUnpackError);
413417

418+
sym_symbolize_keys = ID2SYM(rb_intern("symbolize_keys"));
419+
sym_freeze = ID2SYM(rb_intern("freeze"));
420+
sym_allow_unknown_ext = ID2SYM(rb_intern("allow_unknown_ext"));
421+
414422
rb_define_alloc_func(cMessagePack_Unpacker, MessagePack_Unpacker_alloc);
415423

416424
rb_define_method(cMessagePack_Unpacker, "initialize", MessagePack_Unpacker_initialize, -1);

‎lib/msgpack.rb‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,15 @@
1717

1818
module MessagePack
1919
DefaultFactory = MessagePack::Factory.new
20-
DEFAULT_EMPTY_PARAMS = {}.freeze
2120

2221
def load(src, param = nil)
2322
unpacker = nil
2423

2524
if src.is_a? String
26-
unpacker = DefaultFactory.unpacker param || DEFAULT_EMPTY_PARAMS
25+
unpacker = DefaultFactory.unpacker param
2726
unpacker.feed_reference src
2827
else
29-
unpacker = DefaultFactory.unpacker src, param || DEFAULT_EMPTY_PARAMS
28+
unpacker = DefaultFactory.unpacker src, param
3029
end
3130

3231
unpacker.full_unpack
@@ -36,8 +35,8 @@ def load(src, param = nil)
3635
module_function :load
3736
module_function :unpack
3837

39-
def pack(v, *rest)
40-
packer = DefaultFactory.packer(*rest)
38+
def pack(v, io = nil, options = nil)
39+
packer = DefaultFactory.packer(io, options)
4140
packer.write v
4241
packer.full_pack
4342
end

0 commit comments

Comments
 (0)