From 85a9b97c0fa63bceb76c33df906cf9daded6c159 Mon Sep 17 00:00:00 2001
From: Xavier Noria <fxn@hashref.com>
Date: Sat, 15 Feb 2025 19:54:44 +0100
Subject: [PATCH] Adds code comments

---
 lib/zeitwerk/cref/map.rb | 29 +++++++++++++++++++++++++++++
 lib/zeitwerk/loader.rb   | 12 +++++++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/lib/zeitwerk/cref/map.rb b/lib/zeitwerk/cref/map.rb
index 9ec964e..bac263e 100644
--- a/lib/zeitwerk/cref/map.rb
+++ b/lib/zeitwerk/cref/map.rb
@@ -1,5 +1,34 @@
 # frozen_string_literal: true
 
+# This class emulates a hash table whose keys are of type Zeitwerk::Cref.
+#
+# It is a synchronized hash of hashes. The first one, stored in `@map`, is keyed
+# on class and module object IDs. Then, each one of them stores a hash table
+# keyed on constant names, where we finally store the values.
+#
+# For example, if we store values 0, 1, and 2 for the crefs that would
+# correspond to `M::X`, `M::Y`, and `N::Z`, the map will look like this:
+#
+#   { M => { X: 0, :Y => 1 }, N => { Z: 2 } }
+#
+# Why not use simple hash tables of type Hash[Module, Symbol]? Because class and
+# module objects are not guaranteed to be hashable, the `hash` method may have
+# been overridden:
+#
+#   /~https://github.com/fxn/zeitwerk/issues/188
+#
+# Another option would be to make crefs hashable. I tried with hash code
+#
+#  real_mod_hash(mod) ^ cname.hash
+#
+# and the matching eql?, but that was about 1.8x slower than a hash keyed by
+# class and module names.
+#
+# The gem used hashes keyed by class and module names, but that felt like an
+# unnecessary dependency on said names, our natural objects are the crefs.
+#
+# On the other hand, an unsynchronized hash based on constant paths is 1.6x
+# slower than this map.
 class Zeitwerk::Cref::Map # :nodoc: all
   def initialize
     @map = {}
diff --git a/lib/zeitwerk/loader.rb b/lib/zeitwerk/loader.rb
index a1a1ede..196ba35 100644
--- a/lib/zeitwerk/loader.rb
+++ b/lib/zeitwerk/loader.rb
@@ -88,9 +88,9 @@ class Loader
     # A shadowed file is a file managed by this loader that is ignored when
     # setting autoloads because its matching constant is already taken.
     #
-    # This private set is populated as we descend. For example, if the loader
-    # has only scanned the top-level, `shadowed_files` does not have shadowed
-    # files that may exist deep in the project tree yet.
+    # This private set is populated lazily, as we descend. For example, if the
+    # loader has only scanned the top-level, `shadowed_files` does not have the
+    # shadowed files that may exist deep in the project tree.
     #
     # @sig Set[String]
     attr_reader :shadowed_files
@@ -325,6 +325,9 @@ def cpath_expected_at(path)
     # Says if the given constant path would be unloaded on reload. This
     # predicate returns `false` if reloading is disabled.
     #
+    # This is an undocumented method that I wrote to help transition from the
+    # classic autoloader in Rails. Its usage was removed from Rails in 7.0.
+    #
     # @sig (String) -> bool
     def unloadable_cpath?(cpath)
       unloadable_cpaths.include?(cpath)
@@ -333,6 +336,9 @@ def unloadable_cpath?(cpath)
     # Returns an array with the constant paths that would be unloaded on reload.
     # This predicate returns an empty array if reloading is disabled.
     #
+    # This is an undocumented method that I wrote to help transition from the
+    # classic autoloader in Rails. Its usage was removed from Rails in 7.0.
+    #
     # @sig () -> Array[String]
     def unloadable_cpaths
       to_unload.values.map(&:path)