Skip to content

Commit

Permalink
Replace TracePoint with const_added for explicit namespaces
Browse files Browse the repository at this point in the history
  • Loading branch information
fxn committed Oct 6, 2024
1 parent 8f37a65 commit 92d0dd7
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 249 deletions.
9 changes: 0 additions & 9 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,9 @@ jobs:
- "macos-latest"
- "windows-latest"
ruby-version:
- "2.5"
- "2.6"
- "2.7"
- "3.0"
- "3.1"
- "3.2"
- "3.3"
- "head"
# GitHub is not able to set this combination up lately.
exclude:
- os: "macos-latest"
ruby-version: "2.5"
runs-on: ${{ matrix.os }}
steps:
- uses: "actions/checkout@v4"
Expand Down
34 changes: 10 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
- [Use case: The adapter pattern](#use-case-the-adapter-pattern)
- [Use case: Test files mixed with implementation files](#use-case-test-files-mixed-with-implementation-files)
- [Shadowed files](#shadowed-files)
- [Edge cases](#edge-cases)
- [Edge cases for Zeitwerk < 2.7](#edge-cases-for-zeitwerk--27)
- [Beware of circular dependencies](#beware-of-circular-dependencies)
- [Reopening third-party namespaces](#reopening-third-party-namespaces)
- [Introspection](#introspection)
Expand Down Expand Up @@ -1178,35 +1178,21 @@ file #{file} is ignored because #{constant_path} is already defined

Shadowing only applies to Ruby files, namespace definition can be spread over multiple directories. And you can also reopen third-party namespaces if done [orderly](#reopening-third-party-namespaces).

<a id="markdown-edge-cases" name="edge-cases"></a>
### Edge cases
<a id="markdown-edge-cases-for-zeitwerk--27" name="edge-cases-for-zeitwerk--27"></a>
### Edge cases for Zeitwerk < 2.7

[Explicit namespaces](#explicit-namespaces) like `Trip` here:
In versions of Zeitwerk prior to 2.7, [explicit namespaces](#explicit-namespaces) had to be defined with the `class`/`module` keywords. For technical reasons, direct constant assignment was not supported:

```ruby
# trip.rb
class Trip
include Geolocation
end

# trip/geolocation.rb
module Trip::Geolocation
...
end
# hotel.rb
Hotel = Class { ...} # Not supported in Zeitwerk < 2.7.
Hotel = Struct.new { ... } # Not supported in Zeitwerk < 2.7.
Hotel = Data.define { ... } # Not supported in Zeitwerk < 2.7.
```

have to be defined with the `class`/`module` keywords, as in the example above.

For technical reasons, raw constant assignment is not supported:

```ruby
# trip.rb
Trip = Class { ...} # NOT SUPPORTED
Trip = Struct.new { ... } # NOT SUPPORTED
Trip = Data.define { ... } # NOT SUPPORTED
```
This only affected explicit namespaces, classes and modules that did not act as namespaces were not affected.

This only affects explicit namespaces, those idioms work well for any other ordinary class or module.
Starting with Zeitwerk 2.7, direct constant assignments like those are supported in all cases.

<a id="markdown-beware-of-circular-dependencies" name="beware-of-circular-dependencies"></a>
### Beware of circular dependencies
Expand Down
4 changes: 3 additions & 1 deletion lib/zeitwerk.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ module Zeitwerk
require_relative "zeitwerk/inflector"
require_relative "zeitwerk/gem_inflector"
require_relative "zeitwerk/null_inflector"
require_relative "zeitwerk/kernel"
require_relative "zeitwerk/error"
require_relative "zeitwerk/version"

require_relative "zeitwerk/core_ext/kernel"
require_relative "zeitwerk/core_ext/module"

# This is a dangerous method.
#
# @experimental
Expand Down
12 changes: 12 additions & 0 deletions lib/zeitwerk/core_ext/module.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

module Zeitwerk::ConstAdded
def const_added(cname)
if loader = Zeitwerk::ExplicitNamespace.__loader_for(self, cname)
loader.on_namespace_loaded(const_get(cname, false))
end
super
end

Module.prepend(self)
end
105 changes: 48 additions & 57 deletions lib/zeitwerk/explicit_namespace.rb
Original file line number Diff line number Diff line change
@@ -1,93 +1,84 @@
# frozen_string_literal: true

module Zeitwerk
# Centralizes the logic for the trace point used to detect the creation of
# explicit namespaces, needed to descend into matching subdirectories right
# after the constant has been defined.
# Centralizes the logic needed to descend into matching subdirectories right
# after the constant for an explicit namespace has been defined.
#
# The implementation assumes an explicit namespace is managed by one loader.
# Loaders that reopen namespaces owned by other projects are responsible for
# loading their constant before setup. This is documented.
module ExplicitNamespace # :nodoc: all
# Maps cpaths of explicit namespaces with their corresponding loader.
# Entries are added as the namespaces are found, and removed as they are
# autoloaded.
#
# @sig Hash[String => Zeitwerk::Loader]
@cpaths = {}

class << self
include RealModName
extend Internal

# Maps constant paths that correspond to explicit namespaces according to
# the file system, to the loader responsible for them.
#
# @sig Hash[String, Zeitwerk::Loader]
attr_reader :cpaths
private :cpaths

# @sig Mutex
attr_reader :mutex
private :mutex

# @sig TracePoint
attr_reader :tracer
private :tracer

# Asserts `cpath` corresponds to an explicit namespace for which `loader`
# is responsible.
# Registers `cpath` as being the constant path of an explicit namespace
# managed by `loader`.
#
# @sig (String, Zeitwerk::Loader) -> void
internal def register(cpath, loader)
mutex.synchronize do
cpaths[cpath] = loader
# We check enabled? because, looking at the C source code, enabling an
# enabled tracer does not seem to be a simple no-op.
tracer.enable unless tracer.enabled?
end
@cpaths[cpath] = loader
end

# @sig (String) -> Zeitwerk::Loader?
internal def loader_for(mod, cname)
cpath = mod.equal?(Object) ? cname.name : "#{real_mod_name(mod)}::#{cname}"
@cpaths.delete(cpath)
end

# @sig (Zeitwerk::Loader) -> void
internal def unregister_loader(loader)
cpaths.delete_if { |_cpath, l| l == loader }
disable_tracer_if_unneeded
@cpaths.delete_if { _2.equal?(loader) }
end

# This is an internal method only used by the test suite.
#
# @sig (String) -> bool
internal def registered?(cpath)
cpaths.key?(cpath)
@cpaths[cpath]
end

# This is an internal method only used by the test suite.
#
# @sig () -> void
private def disable_tracer_if_unneeded
mutex.synchronize do
tracer.disable if cpaths.empty?
end
internal def clear
@cpaths.clear
end

# @sig (TracePoint) -> void
private def tracepoint_class_callback(event)
# If the class is a singleton class, we won't do anything with it so we
# can bail out immediately. This is several orders of magnitude faster
# than accessing its name.
return if event.self.singleton_class?
module Synchronized
extend Internal

MUTEX = Mutex.new

# It might be tempting to return if name.nil?, to avoid the computation
# of a hash code and delete call. But Ruby does not trigger the :class
# event on Class.new or Module.new, so that would incur in an extra call
# for nothing.
#
# On the other hand, if we were called, cpaths is not empty. Otherwise
# the tracer is disabled. So we do need to go ahead with the hash code
# computation and delete call.
if loader = cpaths.delete(real_mod_name(event.self))
loader.on_namespace_loaded(event.self)
disable_tracer_if_unneeded
internal def register(...)
MUTEX.synchronize { super }
end
end
end

@cpaths = {}
@mutex = Mutex.new
internal def loader_for(...)
MUTEX.synchronize { super }
end

internal def unregister_loader(...)
MUTEX.synchronize { super }
end

# We go through a method instead of defining a block mainly to have a better
# label when profiling.
@tracer = TracePoint.new(:class, &method(:tracepoint_class_callback))
internal def registered?(...)
MUTEX.synchronize { super }
end

internal def clear
MUTEX.synchronize { super }
end
end

prepend Synchronized unless RUBY_ENGINE == "ruby"
end
end
end
12 changes: 7 additions & 5 deletions lib/zeitwerk/loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ def all_dirs
# Registering is idempotent, and we have to keep the autoload pointing
# to the file. This may run again if more directories are found later
# on, no big deal.
register_explicit_namespace(cref.path)
register_explicit_namespace(cref)
end
# If the existing autoload points to a file, it has to be preserved, if
# not, it is fine as it is. In either case, we do not need to override.
Expand Down Expand Up @@ -515,8 +515,10 @@ def all_dirs

log("earlier autoload for #{cref.path} discarded, it is actually an explicit namespace defined in #{file}") if logger

# Order matters: When Module#const_added is triggered by the autoload, we
# don't want the namespace to be registered yet.
define_autoload(cref, file)
register_explicit_namespace(cref.path)
register_explicit_namespace(cref)
end

# @sig (Module, Symbol, String) -> void
Expand Down Expand Up @@ -549,9 +551,9 @@ def all_dirs
end
end

# @sig (String) -> void
private def register_explicit_namespace(cpath)
ExplicitNamespace.__register(cpath, self)
# @sig (Zeitwerk::Cref) -> void
private def register_explicit_namespace(cref)
ExplicitNamespace.__register(cref.path, self)
end

# @sig (String) -> void
Expand Down
2 changes: 1 addition & 1 deletion lib/zeitwerk/loader/callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module Zeitwerk::Loader::Callbacks
end

# Invoked when a class or module is created or reopened, either from the
# tracer or from module autovivification. If the namespace has matching
# const_added or from module autovivification. If the namespace has matching
# subdirectories, we descend into them now.
#
# @private
Expand Down
Loading

0 comments on commit 92d0dd7

Please sign in to comment.