Load yaml only when a YAML layout or object format is used - #254
Open
tas50 wants to merge 1 commit into
Open
Conversation
logging.rb requires yaml at the top, but YAML is only reachable through
two opt-in paths: Layout#try_yaml, used when :obj_format is :yaml, and
the format method generated by Parseable.create_yaml_format_method,
used by Parseable.yaml layouts.
Everyone else pays for psych, and psych pulls date and stringio along
with it. That is the single largest item in the require graph.
Measured on Ruby 4.0.6 (arm64-darwin), best of seven runs:
require 'logging' files loaded
before 33.21 ms 85
after 21.83 ms 46
-11.38 ms -39 (34% faster)
The require is placed in the two funnels rather than at the point of
use. create_yaml_format_method builds a format method whose body calls
#to_yaml, so yaml has to be loaded before that generated method runs,
not merely before it is defined.
socket and fileutils were considered and left alone. Socket.gethostname
is evaluated while building Parseable::DIRECTIVE_TABLE, so socket is
genuinely needed at load time, and logging/utils.rb reopens FileUtils
to add concat, which would silently define an empty module if fileutils
were not already loaded.
Test suite unchanged at 202 tests, 3 failures, all three pre-existing
on Ruby 4.0.6 and unrelated to YAML (two syslog frozen-string-literal
warnings, one log event method_name assertion).
The four added tests cover requiring the gem, a YAML layout, the :yaml
object format, and YAML event formatting. The first fails against the
previous code.
Signed-off-by: Tim Smith <tsmith84@proton.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
logging.rbrequiresyamlat the top of the file, but YAML is only reachable through two opt-in paths:Layout#try_yaml, used when:obj_formatis:yamlformatmethod generated byParseable.create_yaml_format_method, used byParseable.yamllayoutsEveryone else pays for psych, which drags
dateandstringioalong with it. It is the single largest item in the require graph:Fix
Move the require into the two funnels.
Note the second one is placed in
create_yaml_format_methodrather than at the point of use: that method builds aformatmethod whose body calls#to_yaml, so yaml has to be loaded before the generated method runs, not merely before it is defined.Measurements
Ruby 4.0.6 (arm64-darwin), best of seven runs:
require 'logging'Confirmed directly:
Two things I looked at and deliberately left alone
socket(3.59 ms) —Socket.gethostnameis evaluated while building theParseable::DIRECTIVE_TABLEconstant, so socket really is needed at load time. Deferring it would mean making the hostname lazy, which is a behavior change, not a require move.fileutils(5.54 ms) —logging/utils.rbreopensmodule FileUtilsto addconcat. Without fileutils already loaded that would silently define an emptyFileUtilsmodule andcopy_file/touchwould go missing. Not worth the fragility.Tests
Full suite unchanged: 202 tests, 3 failures, all three pre-existing on Ruby 4.0.6 and unrelated to YAML (two syslog frozen-string-literal warnings, one
test_method_nameassertion). With the added tests: 206 tests, same 3 failures.The four added tests cover requiring the gem, a YAML layout, the
:yamlobject format, and YAML event formatting. The first fails against the previous code.