Lazy-load models and API clients with autoload - #380
Open
ngan wants to merge 1 commit into
Open
Conversation
Requiring the gem loaded all 543 models and 10 API clients for every Xero
API, whether or not the consumer touched them, plus faraday and json-jwt via
api_client.rb. That is ~180k LOC and ~58 MB to call one endpoint, and it is
why adding this gem to a Gemfile without `require: false` is expensive.
Register everything with `autoload` instead -- models, API clients, and the
shared classes. Nothing is referenced at load time: ApiClient#deserialize and
each model's `_deserialize` resolve types through `const_get`, which triggers
autoload transparently, so this is invisible to callers.
Measured on Ruby 3.4.10:
before after
require 'xero-ruby' 57.9 MB 1.0 MB (1249 -> 160 files)
1.29s 0.00s
+ Accounting::Invoice 1.7 MB
+ ApiClient 23.0 MB (faraday + json-jwt)
+ ApiClient & Accounting 31.8 MB
Making the shared classes lazy exposed two latent bugs that eager loading had
been masking, both fixed here: configuration.rb calls Logger.new without
requiring 'logger', and where.rb branches on Date and DateTime without
requiring 'date'. Both previously worked only because api_client.rb was loaded
first and happened to require them.
`configure` moves to a Singleton module that is extended at the bottom of the
entry point, following the pattern in Gusto/fixture_kit.
Autoload paths are absolute, so Ruby resolves them directly instead of
searching $LOAD_PATH. The list mirrors the previous require list rather than
globbing the model directory, keeping the public constant surface unchanged at
559 constants. Note that globbing would also pick up 10 Finance models whose
requires were dropped by the 13.0.0 release but whose files were never
deleted; they have no return_type references and raise NameError today, so
that is preserved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KCuhKDE1ZdU7s6seTURJx3
Contributor
|
Nice one |
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
2 participants
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
require 'xero-ruby'loads all 543 models, 10 API clients, and — viaapi_client.rb— faraday and json-jwt. That's ~180k LOC parsed to call one endpoint, regardless of which API set you use. Adding the gem to a Gemfile withoutrequire: falsecosts ~58 MB of RSS.Change
Register everything in
lib/xero-ruby.rbwithautoload: models, API clients, and the shared classes. There's no load-time coupling to work around —ApiClient#deserializeand each model's_deserializeresolve types throughconst_get, which triggers autoload — so this is invisible to callers.Savings
Ruby 3.4.10, RSS delta from a bare interpreter, best of 3:
require 'xero-ruby'Accounting::InvoiceApiClientApiClient& Accounting APIConsumers who genuinely use every model end up where they started — no regression at the ceiling.
Two latent bugs fixed
Eager loading was masking these. Both surface once the shared classes can load independently:
configuration.rbcallsLogger.newwithoutrequire 'logger'—Configuration.defaultraisedNameError: uninitialized constant XeroRuby::Configuration::Loggerwhere.rbbranches onDate/DateTimewithoutrequire 'date'—Where#to_paramraised on any Array or Range valueBoth worked only because
api_client.rbwas loaded first and happened to require them.Notes
configuremoves to aSingletonmodule extended at the bottom of the entry point.__dir__), so Ruby resolves them directly rather than searching$LOAD_PATH. No double-load against the gemspec's relativerequire "xero-ruby/version", verified in both orders.models/, so the public constant surface is unchanged — 559 constants, diffed before and after.cis_settings.rbdefinesCISSettings,ni_category.rbdefinesNICategory,api_exception.rbdefinesAPIException. Camelizing breaks 10 files.models/would pick up 10Finance::*models whose requires were dropped in 13.0.0 (8c3bd9f) but whose files were never deleted. They have noreturn_typereferences and raiseNameErroron master today. This PR preserves that.Verification
3197 examples, 0 failures.
bundle exec rubocopclean (1030 files).find . -name "*.rb" | xargs -n 1 ruby -cpasses.On generated code
Per CONTRIBUTING,
lib/xero-ruby.rbcomes from your mustache templates, so the durable fix is a template change rather than this diff — the next release would regenerate the file and silently revert to eager loading (nothing breaks, it just gets slow and fat again). Happy to port this to the template instead, or open an issue to discuss first if that's preferred; this PR is mainly here to show the measurements and that the approach is sound.