Actions
Feature #9171
closed
[patch] use fstrings for symbol table
Feature #9171: [patch] use fstrings for symbol table
Description
Here is a simple patch to use fstrings for the table backing symbols.
Unfortunately it causes a segfault in test/rdoc/test_rdoc_parser_ruby.rb. Maybe someone wants to investigate.
diff --git a/parse.y b/parse.y index 8207ad7..b1f3112 100644 --- a/parse.y +++ b/parse.y @@ -10333,7 +10333,7 @@ register_symid(ID id, const char *name, long len, rb_encoding *enc) static ID register_symid_str(ID id, VALUE str) { - OBJ_FREEZE(str); + str = rb_fstring(str); if (RUBY_DTRACE_SYMBOL_CREATE_ENABLED()) { RUBY_DTRACE_SYMBOL_CREATE(RSTRING_PTR(str), rb_sourcefile(), rb_sourceline()); diff --git a/string.c b/string.c index 231bb2f..6ae33e3 100644 --- a/string.c +++ b/string.c @@ -138,6 +138,9 @@ rb_fstring(VALUE str) st_data_t fstr; Check_Type(str, T_STRING); + if (!frozen_strings) + frozen_strings = st_init_table(&fstring_hash_type); + if (FL_TEST(str, RSTRING_FSTR)) return str; @@ -8707,8 +8710,6 @@ Init_String(void) #undef rb_intern #define rb_intern(str) rb_intern_const(str) - frozen_strings = st_init_table(&fstring_hash_type); - rb_cString = rb_define_class("String", rb_cObject); rb_include_module(rb_cString, rb_mComparable); rb_define_alloc_func(rb_cString, empty_str_alloc); 
Updated by hsbt (Hiroshi SHIBATA) almost 12 years ago
- Assignee set to ko1 (Koichi Sasada)
ko1: Could you review this?
Updated by ko1 (Koichi Sasada) almost 12 years ago
- Category set to core
- Assignee changed from ko1 (Koichi Sasada) to nobu (Nobuyoshi Nakada)
Nobu, could you check it?
Updated by avit (Andrew Vit) almost 12 years ago
Would this allow the symbol table to be GC'd or is that a separate issue?
Updated by nobu (Nobuyoshi Nakada) almost 12 years ago
different.
Updated by nobu (Nobuyoshi Nakada) almost 12 years ago
Seems my reply hasn't caught by the ITS...
(13/11/28 16:55), tmm1 (Aman Gupta) wrote:
diff --git a/parse.y b/parse.y index 8207ad7..b1f3112 100644 --- a/parse.y +++ b/parse.y @@ -10333,7 +10333,7 @@ register_symid(ID id, const char *name, long len, rb_encoding *enc) static ID register_symid_str(ID id, VALUE str) { - OBJ_FREEZE(str); + str = rb_fstring(str);
At this point, rb_cString is not created yet.
And unfrozen string will be duplicated in rb_fstring().
diff --git i/parse.y w/parse.y index 8207ad7..9f580e6 100644 --- i/parse.y +++ w/parse.y @@ -10334,6 +10334,7 @@ static ID register_symid_str(ID id, VALUE str) { OBJ_FREEZE(str); + str = rb_fstring(str); if (RUBY_DTRACE_SYMBOL_CREATE_ENABLED()) { RUBY_DTRACE_SYMBOL_CREATE(RSTRING_PTR(str), rb_sourcefile(), rb_sourceline()); diff --git i/string.c w/string.c index 151170c..ec43af7 100644 --- i/string.c +++ w/string.c @@ -165,6 +165,9 @@ rb_fstring(VALUE str) VALUE fstr = Qnil; Check_Type(str, T_STRING); + if (!frozen_strings) + frozen_strings = st_init_table(&fstring_hash_type); + if (FL_TEST(str, RSTRING_FSTR)) return str; @@ -173,6 +176,13 @@ rb_fstring(VALUE str) } static int +fstring_set_class_i(st_data_t key, st_data_t val, st_data_t arg) +{ + RBASIC_SET_CLASS((VALUE)key, (VALUE)arg); + return ST_CONTINUE; +} + +static int fstring_cmp(VALUE a, VALUE b) { int cmp = rb_str_hash_cmp(a, b); @@ -8718,8 +8728,6 @@ Init_String(void) #undef rb_intern #define rb_intern(str) rb_intern_const(str) - frozen_strings = st_init_table(&fstring_hash_type); - rb_cString = rb_define_class("String", rb_cObject); rb_include_module(rb_cString, rb_mComparable); rb_define_alloc_func(rb_cString, empty_str_alloc); @@ -8891,4 +8899,6 @@ Init_String(void) rb_define_method(rb_cSymbol, "swapcase", sym_swapcase, 0); rb_define_method(rb_cSymbol, "encoding", sym_encoding, 0); + + st_foreach(frozen_strings, fstring_set_class_i, rb_cString); } 
Updated by tmm1 (Aman Karmani) almost 12 years ago
Ah great! Thanks for finding my bug.
With your patch, long-lived strings on our rails app heap are reduced by 6000. I think we should merge it.
Updated by tmm1 (Aman Karmani) almost 12 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r44057.
Aman, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
parse.y: use rb_fstring() for strings stored in the symbol table
- parse.y (register_symid_str): use fstrings in symbol table
[Bug #9171] [ruby-core:58656] - parse.y (rb_id2str): ditto
- string.c (rb_fstring): create frozen_strings on first usage. this
allows rb_fstring() calls from the parser (before cString is created) - string.c (fstring_set_class_i): set klass on fstrings generated
before cString was defined - string.c (Init_String): convert frozen_strings table to String
objects after boot - ext/-test-/symbol/type.c (bug_sym_id2str): expose rb_id2str()
- test/-ext-/symbol/test_type.rb (module Test_Symbol): verify symbol
table entries are fstrings
Updated by nobu (Nobuyoshi Nakada) almost 10 years ago
- Tracker changed from Bug to Feature
- Description updated (diff)
Actions