Project

General

Profile

Actions

Feature #10182

closed

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

Added by normalperson (Eric Wong) over 8 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Target version:
[ruby-core:64618]

Description

Cleanup in case MVM development proceeds.


Files

mvm-fstring.patch (3.34 KB) mvm-fstring.patch normalperson (Eric Wong), 08/28/2014 10:51 PM
mvm-fstring.patch (5.77 KB) mvm-fstring.patch nobu (Nobuyoshi Nakada), 08/29/2014 08:39 AM

Updated by nobu (Nobuyoshi Nakada) over 8 years ago

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

Actions #2

Updated by Anonymous over 8 years 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]

Updated by normalperson (Eric Wong) over 8 years ago

wrote:

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

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

Updated by ko1 (Koichi Sasada) over 8 years ago

(2014/08/28 19:51), 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

Updated by normalperson (Eric Wong) over 8 years ago

wrote:

(2014/08/28 19:51), 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))

Updated by ko1 (Koichi Sasada) over 8 years 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

Updated by nobu (Nobuyoshi Nakada) over 8 years ago

What about moving fstring stuffs to vm.c?

Updated by normalperson (Eric Wong) over 8 years ago

wrote:

What about moving fstring stuffs to vm.c?

Your patch looks good to me. ko1?

Updated by ko1 (Koichi Sasada) over 8 years ago

(2014/08/29 5:30), wrote:

What about moving fstring stuffs to vm.c?

I belive fstring codes in string.c.

--
// SASADA Koichi at atdot dot net

Updated by nobu (Nobuyoshi Nakada) over 8 years 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 */

Updated by ko1 (Koichi Sasada) over 8 years ago

(2014/08/29 10:20), wrote:

I belive fstring codes in string.c.
OK.

I like current code.

--
// SASADA Koichi at atdot dot net

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0