diff --git a/src/compiler/crystal/codegen/class_var.cr b/src/compiler/crystal/codegen/class_var.cr index a31a9b00bac5..07d8ed0f96b1 100644 --- a/src/compiler/crystal/codegen/class_var.cr +++ b/src/compiler/crystal/codegen/class_var.cr @@ -25,8 +25,8 @@ class Crystal::CodeGenVisitor initialized_flag_name = class_var_global_initialized_name(class_var) initialized_flag = @main_mod.globals[initialized_flag_name]? unless initialized_flag - initialized_flag = @main_mod.globals.add(@main_llvm_context.int8, initialized_flag_name) - initialized_flag.initializer = @main_llvm_context.int8.const_int(0) + initialized_flag = @main_mod.globals.add(@main_llvm_context.int1, initialized_flag_name) + initialized_flag.initializer = @main_llvm_context.int1.const_int(0) initialized_flag.linkage = LLVM::Linkage::Internal if @single_module initialized_flag.thread_local = true if class_var.thread_local? end @@ -61,7 +61,7 @@ class Crystal::CodeGenVisitor initialized_flag_name = class_var_global_initialized_name(class_var) initialized_flag = @llvm_mod.globals[initialized_flag_name]? unless initialized_flag - initialized_flag = @llvm_mod.globals.add(llvm_context.int8, initialized_flag_name) + initialized_flag = @llvm_mod.globals.add(llvm_context.int1, initialized_flag_name) initialized_flag.thread_local = true if class_var.thread_local? end end diff --git a/src/compiler/crystal/codegen/const.cr b/src/compiler/crystal/codegen/const.cr index decfb3945e20..ec306e97296d 100644 --- a/src/compiler/crystal/codegen/const.cr +++ b/src/compiler/crystal/codegen/const.cr @@ -64,8 +64,8 @@ class Crystal::CodeGenVisitor initialized_flag_name = const.initialized_llvm_name initialized_flag = @main_mod.globals[initialized_flag_name]? unless initialized_flag - initialized_flag = @main_mod.globals.add(@main_llvm_context.int8, initialized_flag_name) - initialized_flag.initializer = @main_llvm_context.int8.const_int(0) + initialized_flag = @main_mod.globals.add(@main_llvm_context.int1, initialized_flag_name) + initialized_flag.initializer = @main_llvm_context.int1.const_int(0) initialized_flag.linkage = LLVM::Linkage::Internal if @single_module end initialized_flag diff --git a/src/compiler/crystal/codegen/once.cr b/src/compiler/crystal/codegen/once.cr index e84dd6b541e0..c14529d80b70 100644 --- a/src/compiler/crystal/codegen/once.cr +++ b/src/compiler/crystal/codegen/once.cr @@ -40,9 +40,6 @@ class Crystal::CodeGenVisitor end state = load(once_state_type, once_state_global) - {% if LibLLVM::IS_LT_150 %} - flag = bit_cast(flag, @llvm_context.int1.pointer) # cast Int8* to Bool* - {% end %} args = [state, flag, initializer] end diff --git a/src/crystal/main.cr b/src/crystal/main.cr index 704153fe13f6..0d5ea638b725 100644 --- a/src/crystal/main.cr +++ b/src/crystal/main.cr @@ -56,7 +56,7 @@ module Crystal # so we explicitly initialize their class vars, then init crystal/once Thread.init Fiber.init - Crystal.once_init + Crystal::Once.init end # :nodoc: diff --git a/src/crystal/once.cr b/src/crystal/once.cr index 87fa9147d56b..f598dd793dfe 100644 --- a/src/crystal/once.cr +++ b/src/crystal/once.cr @@ -7,138 +7,148 @@ # with older compiler releases. It is executed only once at the beginning of the # program and, for the legacy implementation, the result is passed on each call # to `__crystal_once`. -# -# In multithread mode a mutex is used to avoid race conditions between threads. -# -# On Win32, `Crystal::System::FileDescriptor#@@reader_thread` spawns a new -# thread even without the `preview_mt` flag, and the thread can also reference -# Crystal constants, leading to race conditions, so we always enable the mutex. -{% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %} - # This implementation uses an enum over the initialization flag pointer for - # each value to find infinite loops and raise an error. - - module Crystal - # :nodoc: - enum OnceState : Int8 - Processing = -1 - Uninitialized = 0 - Initialized = 1 +require "crystal/pointer_linked_list" +require "crystal/spin_lock" + +module Crystal + # :nodoc: + module Once + struct Operation + include PointerLinkedList::Node + + getter fiber : Fiber + getter flag : Bool* + + def initialize(@flag : Bool*, @fiber : Fiber) + @waiting = PointerLinkedList(Fiber::PointerLinkedListNode).new + end + + def add_waiter(node) : Nil + @waiting.push(node) + end + + def resume_all : Nil + @waiting.each(&.value.enqueue) + end end - {% if flag?(:preview_mt) || flag?(:win32) %} - @@once_mutex = uninitialized Mutex - {% end %} + @@spin = uninitialized SpinLock + @@operations = uninitialized PointerLinkedList(Operation) - # :nodoc: - def self.once_init : Nil - {% if flag?(:preview_mt) || flag?(:win32) %} - @@once_mutex = Mutex.new(:reentrant) - {% end %} + def self.init : Nil + @@spin = SpinLock.new + @@operations = PointerLinkedList(Operation).new end - # :nodoc: - # Using @[NoInline] so LLVM optimizes for the hot path (var already - # initialized). - @[NoInline] - def self.once(flag : OnceState*, initializer : Void*) : Nil - {% if flag?(:preview_mt) || flag?(:win32) %} - @@once_mutex.synchronize { once_exec(flag, initializer) } - {% else %} - once_exec(flag, initializer) - {% end %} + protected def self.exec(flag : Bool*, &) + @@spin.lock + + if flag.value + @@spin.unlock + elsif operation = processing?(flag) + check_reentrancy(operation) + wait_initializer(operation) + else + run_initializer(flag) { yield } + end # safety check, and allows to safely call `Intrinsics.unreachable` in # `__crystal_once` - unless flag.value.initialized? - System.print_error "BUG: failed to initialize constant or class variable\n" - LibC._exit(1) + return if flag.value + + System.print_error "BUG: failed to initialize class variable or constant\n" + LibC._exit(1) + end + + private def self.processing?(flag) + @@operations.each do |operation| + return operation if operation.value.flag == flag end end - private def self.once_exec(flag : OnceState*, initializer : Void*) : Nil - case flag.value - in .initialized? - return - in .uninitialized? - flag.value = :processing - Proc(Nil).new(initializer, Pointer(Void).null).call - flag.value = :initialized - in .processing? + private def self.check_reentrancy(operation) + if operation.value.fiber == Fiber.current + @@spin.unlock raise "Recursion while initializing class variables and/or constants" end end + + private def self.wait_initializer(operation) + waiting = Fiber::PointerLinkedListNode.new(Fiber.current) + operation.value.add_waiter(pointerof(waiting)) + @@spin.unlock + Fiber.suspend + end + + private def self.run_initializer(flag, &) + operation = Operation.new(flag, Fiber.current) + @@operations.push pointerof(operation) + @@spin.unlock + + yield + + @@spin.lock + flag.value = true + @@operations.delete pointerof(operation) + @@spin.unlock + + operation.resume_all + end end # :nodoc: # - # Using `@[AlwaysInline]` allows LLVM to optimize const accesses. Since this - # is a `fun` the function will still appear in the symbol table, though it - # will never be called. - @[AlwaysInline] - fun __crystal_once(flag : Crystal::OnceState*, initializer : Void*) : Nil - return if flag.value.initialized? - - Crystal.once(flag, initializer) + # Never inlined to avoid bloating the call site with the slow-path that should + # usually not be taken. + @[NoInline] + def self.once(flag : Bool*, initializer : Void*) + Once.exec(flag, &Proc(Nil).new(initializer, Pointer(Void).null)) + end - # tell LLVM that it can optimize away repeated `__crystal_once` calls for - # this global (e.g. repeated access to constant in a single funtion); - # this is truly unreachable otherwise `Crystal.once` would have panicked - Intrinsics.unreachable unless flag.value.initialized? + # :nodoc: + # + # NOTE: should also never be inlined, but that would capture the block, which + # would be a breaking change when we use this method to protect class getter + # and class property macros with lazy initialization (the block may return or + # break). + # + # TODO: consider a compile time flag to enable/disable the capture? returning + # from the block is unexpected behavior: the returned value won't be saved in + # the class variable. + def self.once(flag : Bool*, &) + Once.exec(flag) { yield } unless flag.value end -{% else %} - # This implementation uses a global array to store the initialization flag - # pointers for each value to find infinite loops and raise an error. - - module Crystal - # :nodoc: - class OnceState - @rec = [] of Bool* - - @[NoInline] - def once(flag : Bool*, initializer : Void*) - unless flag.value - if @rec.includes?(flag) - raise "Recursion while initializing class variables and/or constants" - end - @rec << flag - - Proc(Nil).new(initializer, Pointer(Void).null).call - flag.value = true - - @rec.pop - end - end +end - {% if flag?(:preview_mt) || flag?(:win32) %} - @mutex = Mutex.new(:reentrant) - - @[NoInline] - def once(flag : Bool*, initializer : Void*) - unless flag.value - @mutex.synchronize do - previous_def - end - end - end - {% end %} - end +{% if compare_versions(Crystal::VERSION, "1.16.0-dev") >= 0 %} + # :nodoc: + # + # We always inline this accessor to optimize for the fast-path (already + # initialized). + @[AlwaysInline] + fun __crystal_once(flag : Bool*, initializer : Void*) + return if flag.value + Crystal.once(flag, initializer) - # :nodoc: - def self.once_init : Nil - end + # tells LLVM to assume that the flag is true, this avoids repeated access to + # the same constant or class variable to check the flag and try to run the + # initializer (only the first access will) + Intrinsics.unreachable unless flag.value end - +{% else %} # :nodoc: + # + # Unused. Kept for backward compatibility with older compilers. fun __crystal_once_init : Void* - Crystal::OnceState.new.as(Void*) + Pointer(Void).null end # :nodoc: @[AlwaysInline] fun __crystal_once(state : Void*, flag : Bool*, initializer : Void*) return if flag.value - state.as(Crystal::OnceState).once(flag, initializer) + Crystal.once(flag, initializer) Intrinsics.unreachable unless flag.value end {% end %} diff --git a/src/crystal/spin_lock.cr b/src/crystal/spin_lock.cr index 105c235e0c66..277fc39be916 100644 --- a/src/crystal/spin_lock.cr +++ b/src/crystal/spin_lock.cr @@ -3,12 +3,12 @@ struct Crystal::SpinLock private UNLOCKED = 0 private LOCKED = 1 - {% if flag?(:preview_mt) %} + {% if flag?(:preview_mt) || flag?(:win32) %} @m = Atomic(Int32).new(UNLOCKED) {% end %} def lock - {% if flag?(:preview_mt) %} + {% if flag?(:preview_mt) || flag?(:win32) %} while @m.swap(LOCKED, :acquire) == LOCKED while @m.get(:relaxed) == LOCKED Intrinsics.pause @@ -18,7 +18,7 @@ struct Crystal::SpinLock end def unlock - {% if flag?(:preview_mt) %} + {% if flag?(:preview_mt) || flag?(:win32) %} @m.set(UNLOCKED, :release) {% end %} end diff --git a/src/prelude.cr b/src/prelude.cr index f84bb86cb3c1..267bb7eb1f48 100644 --- a/src/prelude.cr +++ b/src/prelude.cr @@ -8,10 +8,10 @@ # appear in the API docs. # This list requires ordered statements -require "crystal/once" require "lib_c" require "macros" require "object" +require "crystal/once" require "comparable" require "exception" require "iterable"