Merge pull request #2357 from natalie-lang/args-stack

Store args on a per-Fiber stack

.gitignore

@@ -5,6 +5,7 @@ coverage-report
 .cache
 bin/nat
 /rubyspec_temp
+/master
 
 # Editor tools
 .ccls

Gemfile.lock

@@ -42,6 +42,7 @@ PLATFORMS
   arm64-darwin-21
   arm64-darwin-22
   arm64-darwin-23
+  arm64-darwin-24
   x86_64-darwin-20
   x86_64-darwin-21
   x86_64-darwin-22

Rakefile

@@ -77,7 +77,7 @@ end
 
 desc 'Run the most-recently-modified test when any source files change (requires entr binary)'
 task :watch do
-  files = Rake::FileList['**/*.cpp', '**/*.c', '**/*.hpp', '**/*.rb'].exclude('{build,ext}/**/*')
+  files = Rake::FileList['**/*.cpp', '**/*.c', '**/*.hpp', '**/*.rb'].exclude('{build,ext,master}/**/*')
   sh "ls #{files} | entr -c -s 'rake test_last_modified'"
 end
 
@@ -581,15 +581,15 @@ file 'bin/nat' => OBJECT_FILES + ['bin/natalie'] do
   sh 'bin/natalie -c bin/nat bin/natalie'
 end
 
-file "build/libnat.#{SO_EXT}" => SOURCES + ['lib/libnat_api.rb', 'lib/libnat_api.cpp', 'build/libnatalie.a'] do |t|
+file "build/libnat.#{SO_EXT}" => SOURCES + ['lib/libnat_api.rb', 'lib/libnat_api.cpp'] do |t|
   sh 'bin/natalie --write-obj build/libnat.rb.cpp lib/libnat_api.rb'
   if system('pkg-config --exists libffi')
     flags = `pkg-config --cflags --libs libffi`.chomp
   end
   sh "#{cxx} #{cxx_flags.join(' ')} #{flags} -std=#{STANDARD} " \
      '-DNAT_OBJECT_FILE -shared -fPIC -rdynamic ' \
      '-Wl,-undefined,dynamic_lookup ' \
-     "-o #{t.name} build/libnat.rb.cpp build/libnatalie.a"
+     "-o #{t.name} build/libnat.rb.cpp"
 end
 
 rule '.c.o' => 'src/%n' do |t|

include/natalie/args.hpp

@@ -17,37 +17,38 @@ class HashObject;
 class SymbolObject;
 class Value;
 
+extern thread_local Vector<Value> *tl_current_arg_stack;
+
 class Args {
 public:
     Args() { }
-
     Args(size_t size, const Value *data, bool has_keyword_hash = false);
-
-    Args(const TM::Vector<Value> &vec, bool has_keyword_hash = false)
-        : m_data { vec }
-        , m_has_keyword_hash { has_keyword_hash } { }
-
-    Args(TM::Vector<Value> &&vec, bool has_keyword_hash = false)
-        : m_data { std::move(vec) }
-        , m_has_keyword_hash { has_keyword_hash } { }
-
+    Args(const TM::Vector<Value> &vec, bool has_keyword_hash = false);
     Args(ArrayObject *array, bool has_keyword_hash = false);
-
-    Args(std::initializer_list<Value> args, bool has_keyword_hash = false)
-        : m_data { args }
-        , m_has_keyword_hash { has_keyword_hash } { }
-
-    Args(const Args &other);
+    Args(std::initializer_list<Value> args, bool has_keyword_hash = false);
+    Args(Args &other);
 
     Args(Args &&other)
-        : m_data { std::move(other.m_data) }
+        : m_args_original_start_index { other.m_args_original_start_index }
+        , m_args_start_index { other.m_args_start_index }
+        , m_args_size { other.m_args_size }
         , m_has_keyword_hash { other.m_has_keyword_hash } {
+        other.m_moved_out = true;
         other.m_has_keyword_hash = false;
     }
 
-    Args &operator=(const Args &other);
+    ~Args() {
+        if (!m_moved_out)
+            tl_current_arg_stack->set_size(m_args_original_start_index);
+    }
+
+    Args &operator=(const Args &other) = delete;
 
     Value shift();
+    Value pop();
+
+    Value first() const;
+    Value last() const;
 
     Value operator[](size_t index) const;
 
@@ -61,11 +62,10 @@ class Args {
     void ensure_argc_between(Env *env, size_t expected_low, size_t expected_high, std::initializer_list<const String> keywords = {}) const;
     void ensure_argc_at_least(Env *env, size_t expected, std::initializer_list<const String> keywords = {}) const;
 
-    size_t size() const { return m_data.size(); }
-    const Value *data() const { return m_data.data(); }
+    size_t start_index() const { return m_args_start_index; }
+    size_t size() const { return m_args_size; }
 
-    Vector<Value> &vector() { return m_data; }
-    const Vector<Value> &vector() const { return m_data; }
+    Value *data() const;
 
     bool has_keyword_hash() const { return m_has_keyword_hash; }
     HashObject *keyword_hash() const;
@@ -79,7 +79,10 @@ class Args {
 
     String argc_error_suffix(std::initializer_list<const String> keywords) const;
 
-    Vector<Value> m_data {};
+    size_t m_args_original_start_index { tl_current_arg_stack->size() };
+    size_t m_args_start_index { tl_current_arg_stack->size() };
+    size_t m_args_size { 0 };
+    bool m_moved_out { false };
     bool m_has_keyword_hash { false };
 };
 };

include/natalie/fiber_object.hpp

@@ -16,6 +16,7 @@
 #include "natalie/nil_object.hpp"
 #include "natalie/object.hpp"
 #include "natalie/symbol_object.hpp"
+#include "natalie/thread_object.hpp"
 
 extern "C" {
 void fiber_wrapper_func(mco_coro *co);
@@ -28,6 +29,8 @@ typedef struct {
 
 namespace Natalie {
 
+extern thread_local Vector<Value> *tl_current_arg_stack;
+
 class FiberObject : public Object {
 public:
     enum class Status {
@@ -53,8 +56,6 @@ class FiberObject : public Object {
         mco_destroy(m_coroutine);
     }
 
-    static FiberObject *build_main_fiber(ThreadObject *, void *);
-
     constexpr static int STACK_SIZE = 1024 * 1024;
 
     FiberObject *initialize(Env *env, Value, Value, Block *block);
@@ -75,7 +76,7 @@ class FiberObject : public Object {
     static Value set_scheduler(Env *, Value);
     Value set_storage(Env *, Value);
     Value storage(Env *) const;
-    void swap_to_previous(Env *env, Args &&args);
+    void swap_to_previous(Env *env, size_t arg_size, Value *arg_data);
 
     void *start_of_stack() { return m_start_of_stack; }
 
@@ -113,11 +114,11 @@ class FiberObject : public Object {
             m_start_of_stack);
     }
 
-    static FiberObject *current();
-    static FiberObject *main();
+    static FiberObject *current() { return tl_current_thread->current_fiber(); }
+    static FiberObject *main() { return tl_current_thread->main_fiber(); }
 
     Vector<Value> &args() { return m_args; }
-    void set_args(Args &&args);
+    void set_args(size_t arg_size, Value *arg_data);
 
     ExceptionObject *error() { return m_error; }
     void set_error(ExceptionObject *error) { m_error = error; }
@@ -126,6 +127,10 @@ class FiberObject : public Object {
     HashObject *ensure_thread_storage();
 
 private:
+    friend Args;
+    friend ThreadObject;
+
+    TM::Vector<Value> m_args_stack { 100 };
     Block *m_block { nullptr };
     bool m_blocking { false };
     HashObject *m_storage { nullptr };
@@ -140,7 +145,7 @@ class FiberObject : public Object {
     Status m_status { Status::Created };
     TM::Optional<TM::String> m_file {};
     TM::Optional<size_t> m_line {};
-    Vector<Value> m_args {};
+    TM::Vector<Value> m_args {};
     FiberObject *m_previous_fiber { nullptr };
     ExceptionObject *m_error { nullptr };
 };

include/natalie/module_object.hpp

@@ -148,6 +148,7 @@ class ModuleObject : public Object {
     Value private_class_method(Env *, Args &&);
     Value public_class_method(Env *, Args &&);
     void set_method_visibility(Env *, Args &&, MethodVisibility);
+    void set_method_visibility(Env *, SymbolObject *, MethodVisibility);
     Value module_function(Env *, Args &&) override;
 
     Value deprecate_constant(Env *, Args &&);

include/natalie/thread_object.hpp

@@ -65,7 +65,8 @@ class ThreadObject : public Object {
         detach();
     }
 
-    static void build_main_thread(Env *env, void *start_of_stack);
+    static void prepare_main_thread();
+    static void finish_main_thread_setup(Env *env, void *start_of_stack);
 
     ThreadObject *initialize(Env *env, Args &&args, Block *block);
 
@@ -96,7 +97,7 @@ class ThreadObject : public Object {
     void set_exception(ExceptionObject *exception) { m_exception = exception; }
     ExceptionObject *exception() { return m_exception; }
 
-    ArrayObject *args() { return m_args.to_array(); }
+    ArrayObject *args() { return new ArrayObject(m_args.size(), m_args.data()); }
     Block *block() { return m_block; }
 
     bool is_alive() const {
@@ -277,7 +278,7 @@ class ThreadObject : public Object {
 
     friend FiberObject;
 
-    Args m_args {};
+    TM::Vector<Value> m_args;
     Block *m_block { nullptr };
     std::thread m_thread {};
     std::thread::native_handle_type m_native_thread_handle { 0 };

lib/natalie/compiler/backends/cpp_backend.rb

@@ -33,8 +33,7 @@ class CppBackend
         -lonigmo
       ].freeze
 
-      # When using the REPL or compiling a binary with the `-c` option,
-      # we use static linking for compatibility.
+      # When compiling a binary with the `-c` option, we use static linking for compatibility.
       LIBRARIES_FOR_STATIC_LINKING = %w[
         -lnatalie
       ].freeze
@@ -88,7 +87,7 @@ def compiler_command
         [
           cc,
           build_flags,
-          (shared? ? '-fPIC -shared' : ''),
+          (@compiler.repl? ? LIBNAT_AND_REPL_FLAGS.join(' ') : ''),
           inc_paths.map { |path| "-I #{path}" }.join(' '),
           "-o #{@compiler.out_path}",
           '-x c++ -std=c++17',
@@ -156,7 +155,9 @@ def template
       end
 
       def libraries
-        if @compiler.dynamic_linking?
+        if @compiler.repl?
+          []
+        elsif @compiler.dynamic_linking?
           LIBRARIES_FOR_DYNAMIC_LINKING
         else
           LIBRARIES_FOR_STATIC_LINKING
@@ -266,10 +267,6 @@ def write_object_file?
         !!@compiler.write_obj_path
       end
 
-      def shared?
-        @compiler.repl?
-      end
-
       def declarations
         [
           object_file_declarations,

lib/natalie/compiler/flags.rb

@@ -34,6 +34,13 @@ module Flags
         -fprofile-arcs
         -ftest-coverage
       ].freeze
+
+      LIBNAT_AND_REPL_FLAGS = %w[
+        -fPIC
+        -shared
+        -rdynamic
+        -Wl,-undefined,dynamic_lookup
+      ]
     end
   end
 end