Feature #10182: [PATCH] string.c: move frozen_strings table to rb_vm_t

• Status: Closed
• Assignee: normalperson (Eric Wong)
• Target version: 2.2.0

[normalperson (Eric Wong)]

Cleanup in case MVM development proceeds.

[nobu (Nobuyoshi Nakada)]

The dependency of string.o in common.mk will need $(VM_CORE_H_INCLUDES).

• % Done changed from 0 to 100
• Status changed from Open to Closed

Applied in changeset r47310.

---

string.c: move frozen_strings table to rb_vm_t

Cleanup in case MVM development proceeds.

• string.c: remove static frozen_strings
• string.c (Init_frozen_strings): new function
• string.c (rb_fstring): remove check for frozen strings,
  use per-VM table
• string.c (rb_str_free): use per-VM table
• string.c (Init_String): use per-VM table
• vm_core.h (rb_vm_t): add frozen_strings table
• internal.h (Init_frozen_strings): new function prototype
• eval.c (ruby_setup): call Init_frozen_strings
  [Feature #10182]

[normalperson (Eric Wong)]

[e-mail] wrote:

The dependency of string.o in common.mk will need $(VM_CORE_H_INCLUDES).

Thanks, r47310.
I've been spoiled by automake :x

[ko1 (Koichi Sasada)]

(2014/08/28 19:51), [e-mail] wrote:

Cleanup in case MVM development proceeds.

Now, mvm is stopping.

I don't like to include vm_core.h only for such purpose. It extends
dependency, taking long build time, and so on.

How about to make a function such as "rb_vm_fstring_table()" in string.c
and use C's global variable just now, and comment to take care?

for mvm, in this case, it should use VM global variable accessor
function with specific ID (not available in trunk).

--
// SASADA Koichi at atdot dot net

[normalperson (Eric Wong)]

[e-mail] wrote:

(2014/08/28 19:51), [e-mail] wrote:

Cleanup in case MVM development proceeds.

Now, mvm is stopping.

I am sad :*(

I don't like to include vm_core.h only for such purpose. It extends
dependency, taking long build time, and so on.

OK, I was thinking about making it smaller/modular, too.

How about to make a function such as "rb_vm_fstring_table()" in string.c
and use C's global variable just now, and comment to take care?

OK, I made r47312. I kept the initialization changes since I think
it was ugly to do lazy st_init_table (in case we need thread-safety
in the future, using something like rculfhash (from Userspace-RCU))

[ko1 (Koichi Sasada)]

(2014/08/29 4:24), Eric Wong wrote:

OK, I made r47312. I kept the initialization changes since I think
it was ugly to do lazy st_init_table (in case we need thread-safety
in the future, using something like rculfhash (from Userspace-RCU))

reasonable.

--
// SASADA Koichi at atdot dot net

[nobu (Nobuyoshi Nakada)]

What about moving fstring stuffs to vm.c?

[nobu (Nobuyoshi Nakada)]

• File mvm-fstring.patch mvm-fstring.patch added

[normalperson (Eric Wong)]

[e-mail] wrote:

What about moving fstring stuffs to vm.c?

Your patch looks good to me. ko1?

[ko1 (Koichi Sasada)]

(2014/08/29 5:30), [e-mail] wrote:

What about moving fstring stuffs to vm.c?

I belive fstring codes in string.c.

--
// SASADA Koichi at atdot dot net

[nobu (Nobuyoshi Nakada)]

Koichi Sasada wrote:

I belive fstring codes in string.c.

OK.

diff --git i/eval.c w/eval.c
index f0a06fb..fbe17d0 100644
--- i/eval.c
+++ w/eval.c
@@ -52,7 +52,6 @@ ruby_setup(void)
     Init_BareVM();
     Init_heap();
     Init_vm_objects();
-    Init_frozen_strings();
 
     PUSH_TAG();
     if ((state = EXEC_TAG()) == 0) {
diff --git i/string.c w/string.c
index e7a971b..0b1540b 100644
--- i/string.c
+++ w/string.c
@@ -176,16 +176,9 @@ mustnot_broken(VALUE str)
 
 static int fstring_cmp(VALUE a, VALUE b);
 
-/* in case we restart MVM development, this needs to be per-VM */
-static st_table* frozen_strings;
+st_table *rb_vm_fstring_table(void);
 
-static inline st_table*
-rb_vm_fstring_table(void)
-{
-    return frozen_strings;
-}
-
-static const struct st_hash_type fstring_hash_type = {
+const struct st_hash_type rb_fstring_hash_type = {
     fstring_cmp,
     rb_str_hash,
 };
@@ -8955,13 +8948,5 @@ Init_String(void)
 
     rb_define_method(rb_cSymbol, "encoding", sym_encoding, 0);
 
-    assert(rb_vm_fstring_table());
     st_foreach(rb_vm_fstring_table(), fstring_set_class_i, rb_cString);
 }
-
-void
-Init_frozen_strings(void)
-{
-    assert(!frozen_strings);
-    frozen_strings = st_init_table(&fstring_hash_type);
-}
diff --git i/vm.c w/vm.c
index cd80729..0251c9e 100644
--- i/vm.c
+++ w/vm.c
@@ -2787,6 +2787,14 @@ Init_BareVM(void)
     ruby_thread_init_stack(th);
 }
 
+st_table *
+rb_vm_fstring_table(void)
+{
+    return GET_VM()->frozen_strings;
+}
+
+extern const struct st_hash_type rb_fstring_hash_type;
+
 void
 Init_vm_objects(void)
 {
@@ -2796,6 +2804,8 @@ Init_vm_objects(void)
 
     /* initialize mark object array, hash */
     vm->mark_object_ary = rb_ary_tmp_new(128);
+
+    vm->frozen_strings = st_init_table(&rb_fstring_hash_type);
 }
 
 /* top self */

[ko1 (Koichi Sasada)]

(2014/08/29 10:20), [e-mail] wrote:

I belive fstring codes in string.c.
OK.

I like current code.

--
// SASADA Koichi at atdot dot net