Feature #10182

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

Added by Eric Wong 9 months ago. Updated 9 months ago.

[ruby-core:64618]
Status:Closed
Priority:Low
Assignee:Eric Wong

Description

Cleanup in case MVM development proceeds.

mvm-fstring.patch Magnifier (3.34 KB) Eric Wong, 08/28/2014 10:51 PM

mvm-fstring.patch Magnifier (5.77 KB) Nobuyoshi Nakada, 08/29/2014 08:39 AM

Associated revisions

Revision 47310
Added by normal 9 months ago

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]

Revision 47310
Added by normal 9 months ago

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]

History

#1 Updated by Nobuyoshi Nakada 9 months ago

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

#2 Updated by Anonymous 9 months ago

  • % 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]

#3 Updated by Eric Wong 9 months ago

nobu@ruby-lang.org wrote:

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

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

#4 Updated by Koichi Sasada 9 months ago

(2014/08/28 19:51), normalperson@yhbt.net 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

#5 Updated by Eric Wong 9 months ago

ko1@atdot.net wrote:

(2014/08/28 19:51), normalperson@yhbt.net 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))

#6 Updated by Koichi Sasada 9 months ago

(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

#7 Updated by Nobuyoshi Nakada 9 months ago

What about moving fstring stuffs to vm.c?

#8 Updated by Nobuyoshi Nakada 9 months ago

#9 Updated by Eric Wong 9 months ago

nobu@ruby-lang.org wrote:

What about moving fstring stuffs to vm.c?

Your patch looks good to me. ko1?

#10 Updated by Koichi Sasada 9 months ago

(2014/08/29 5:30), nobu@ruby-lang.org wrote:

What about moving fstring stuffs to vm.c?

I belive fstring codes in string.c.

--
// SASADA Koichi at atdot dot net

#11 Updated by Nobuyoshi Nakada 9 months ago

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 */

#12 Updated by Koichi Sasada 9 months ago

(2014/08/29 10:20), nobu@ruby-lang.org wrote:

I belive fstring codes in string.c.
OK.

I like current code.

--
// SASADA Koichi at atdot dot net

Also available in: Atom PDF