Forum: Ruby-core [ruby-trunk - Bug #9171][Open] [patch] use fstrings for symbol table

398856ea967f3cab2dbe3df99d732069?d=identicon&s=25 tmm1 (Aman Gupta) (Guest)
on 2013-11-28 08:56
(Received via mailing list)
Issue #9171 has been reported by tmm1 (Aman Gupta).

----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee:
Category:
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
Eabad423977cfc6873b8f5df62b848a6?d=identicon&s=25 hsbt (Hiroshi SHIBATA) (Guest)
on 2013-11-29 10:47
(Received via mailing list)
Issue #9171 has been updated by hsbt (Hiroshi SHIBATA).

Assignee set to ko1 (Koichi Sasada)

ko1: Could you review this?
----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43246

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category:
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
C4e88907313843cf07f6d85ba8162120?d=identicon&s=25 ko1 (Koichi Sasada) (Guest)
on 2013-12-06 11:39
(Received via mailing list)
Issue #9171 has been updated by ko1 (Koichi Sasada).

Category set to core
Assignee changed from ko1 (Koichi Sasada) to nobu (Nobuyoshi Nakada)

Nobu, could you check it?

----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43459

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (nobu)
on 2013-12-06 13:50
(Received via mailing list)
(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);
 }
F0987c97234fa9c6b26f796bdbdab037?d=identicon&s=25 avit (Andrew Vit) (Guest)
on 2013-12-06 22:17
(Received via mailing list)
Issue #9171 has been updated by avit (Andrew Vit).


Would this allow the symbol table to be GC'd or is that a separate
issue?
----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43470

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (nobu)
on 2013-12-07 01:22
(Received via mailing list)
Issue #9171 has been updated by nobu (Nobuyoshi Nakada).


different.
----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43476

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (nobu)
on 2013-12-07 01:23
(Received via mailing list)
Issue #9171 has been updated by nobu (Nobuyoshi Nakada).


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);
 }


----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43477

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
398856ea967f3cab2dbe3df99d732069?d=identicon&s=25 tmm1 (Aman Gupta) (Guest)
on 2013-12-07 11:38
(Received via mailing list)
Issue #9171 has been updated by tmm1 (Aman Gupta).


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.
----------------------------------------
Bug #9171: [patch] use fstrings for symbol table
https://bugs.ruby-lang.org/issues/9171#change-43494

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-11-27 trunk 43880)
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


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);
This topic is locked and can not be replied to.