From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 012041FBEC for ; Sun, 5 Mar 2017 02:26:35 +0000 (UTC) From: Eric Wong To: kcar-public@bogomips.org Subject: [PATCH] rely on String#-@ (str_uminus) to dedupe headers Date: Sun, 5 Mar 2017 02:26:35 +0000 Message-Id: <20170305022635.12411-1-e@80x24.org> List-Id: At least for Ruby 2.5.0 (due Dec 2017), this will reduce memory overhead when dealing with common headers. There is a small behavior change for people using Arrays, as they get deduplicate strings; so we'll need a version bump. --- ext/kcar/extconf.rb | 11 +++++++++++ ext/kcar/kcar.rl | 17 +++++++++++++---- test/test_parser.rb | 17 +++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/ext/kcar/extconf.rb b/ext/kcar/extconf.rb index 218c2e3..89ac586 100644 --- a/ext/kcar/extconf.rb +++ b/ext/kcar/extconf.rb @@ -6,4 +6,15 @@ dir_config("kcar") have_macro("SIZEOF_OFF_T", "ruby.h") or check_sizeof("off_t", "sys/types.h") have_macro("SIZEOF_LONG", "ruby.h") or check_sizeof("long", "sys/types.h") +uminus_dedupe = false +begin + # oddly, opt_str_freeze is not always effective: + # https://bugs.ruby-lang.org/issues/13282 + a = -(%w(t e s t).join) + b = -(%w(t e s t).join) + uminus_dedupe = a.object_id == b.object_id +rescue NoMethodError +end +$CFLAGS += " -DSTR_UMINUS_DEDUPE=#{uminus_dedupe ? 1 : 0}" + create_makefile("kcar_ext") diff --git a/ext/kcar/kcar.rl b/ext/kcar/kcar.rl index e015bdb..cbffb7e 100644 --- a/ext/kcar/kcar.rl +++ b/ext/kcar/kcar.rl @@ -14,7 +14,7 @@ #include "c_util.h" static VALUE eParserError; -static ID id_sq, id_sq_set; +static ID id_uminus, id_sq, id_sq_set; /** Defines common length and error messages for input length validation. */ #define DEF_MAX_LENGTH(N, length) \ @@ -95,6 +95,15 @@ static int is_lws(char c) return (c == ' ' || c == '\t'); } +/* this will dedupe under Ruby 2.5+ (December 2017) */ +static VALUE str_dd_freeze(VALUE str) +{ + if (STR_UMINUS_DEDUPE) + return rb_funcall(str, id_uminus, 0); + OBJ_FREEZE(str); + return str; +} + static VALUE stripped_str_new(const char *str, long len) { long end; @@ -150,7 +159,7 @@ status_phrase(struct http_parser *hp, VALUE hdr, const char *ptr, size_t len) { long nr; - hp->status = rb_str_new(ptr, len); + hp->status = str_dd_freeze(rb_str_new(ptr, len)); /* RSTRING_PTR is null terminated, ptr is not */ nr = strtol(RSTRING_PTR(hp->status), NULL, 10); @@ -227,7 +236,7 @@ static void write_value(VALUE hdr, struct http_parser *hp, vlen = LEN(mark, p); VALIDATE_MAX_LENGTH(vlen, FIELD_VALUE); VALIDATE_MAX_LENGTH(flen, FIELD_NAME); - f = rb_str_new(fptr, (long)flen); + f = str_dd_freeze(rb_str_new(fptr, (long)flen)); v = stripped_str_new(vptr, (long)vlen); /* needs more tests for error-checking here */ @@ -286,7 +295,6 @@ static void write_value(VALUE hdr, struct http_parser *hp, if (NIL_P(e)) { /* new value, freeze it since it speeds up MRI slightly */ - OBJ_FREEZE(f); if (hclass == rb_cHash) rb_hash_aset(hdr, f, v); @@ -740,4 +748,5 @@ void Init_kcar_ext(void) rb_define_const(cParser, "LENGTH_MAX", OFFT2NUM(UH_OFF_T_MAX)); id_sq = rb_intern("[]"); id_sq_set = rb_intern("[]="); + id_uminus = rb_intern("-@"); } diff --git a/test/test_parser.rb b/test/test_parser.rb index c0a05f1..3768808 100644 --- a/test/test_parser.rb +++ b/test/test_parser.rb @@ -303,4 +303,21 @@ class TestParser < Test::Unit::TestCase rescue LoadError warn 'ObjectSpace not available' end + + def test_uminus_dd + # oddly, opt_str_freeze is not always effective: + # https://bugs.ruby-lang.org/issues/13282 + a = -(%w(H o s t).join) + b = -(%w(H o s t).join) + if a.object_id == b.object_id + resp = "HTTP/1.1 200 OK\r\nHost: example.com\r\n\r\n" + assert @hp.headers(e = {}, resp.dup) + @hp.reset + assert @hp.headers(f = {}, resp.dup) + assert_same e.keys[0], f.keys[0] + assert_same a, e.keys[0] + else + warn "String#-@ does not dedupe with #{RUBY_ENGINE}-#{RUBY_VERSION}" + end + end end -- EW