Skip to content

perf: avoid test_inverse import-time compilation#62

Open
Beforerr wants to merge 1 commit into
JuliaMath:masterfrom
Beforerr:perf/test-inverse-load-time
Open

perf: avoid test_inverse import-time compilation#62
Beforerr wants to merge 1 commit into
JuliaMath:masterfrom
Beforerr:perf/test-inverse-load-time

Conversation

@Beforerr
Copy link
Copy Markdown

@Beforerr Beforerr commented May 18, 2026

Summary

Small load-time cleanup: replace the runtime __init__ error hint with a direct test_inverse fallback method. Users still get a friendly error when Test is missing, but package import no longer pays for the error-hint hook.

Benchmark

Measured with julia --startup-file=no --project=@. -e 'using InteractiveUtils; @time_imports using InverseFunctions' after precompile.

  • before: 74.2 ms InverseFunctions, 98.73% compilation time
  • after: 0.7 ms InverseFunctions

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.49%. Comparing base (bbb49fe) to head (441dcba).

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
+ Coverage   98.48%   98.49%   +0.01%     
==========================================
  Files           6        6              
  Lines         132      133       +1     
==========================================
+ Hits          130      131       +1     
  Misses          2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Beforerr Beforerr force-pushed the perf/test-inverse-load-time branch from 8bc2d9c to 70c8b41 Compare May 18, 2026 02:46
Comment thread src/InverseFunctions.jl
Comment on lines +29 to +31
function test_inverse(args...; kwargs...)
throw(ArgumentError("InverseFunctions.test_inverse requires the Test standard library. Did you forget to load Test?"))
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the behaviour: Currently, the error hint with the question is only shown when the extension is actually not loaded. This message here will instead be displayed unconditionally, and users will see an ArgumentError with an incorrect message instead of eg a MethodError.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I changed this so the fallback no longer changes unrelated call errors.

The fallback is still a varargs method so the Test extension can add its real test_inverse(f, x; ...) method without overwriting during precompilation. But it now only emits the friendly missing-Test message for the normal two-argument call. Other signatures throw MethodError, matching existing behavior better.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, even the behaviour of the new version is different from master. There's no point in asking the user to load the extension if it's already loaded.

Copy link
Copy Markdown
Author

@Beforerr Beforerr May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If test is already loaded, the ArgumentError would never be throwed I think.

Since Julia owns MethodError display so to improve load-time compilation,
options are :

  1. Exact old exception behavior, faster load: MethodError, but no “Did you forget…” hint.
  2. Or friendly message, changed exception: define fallback method, throw ArgumentError / ErrorException.

I felt like keeping original UX with longer load-time is worse as this function is rarely used downstream and require Test to load first to use it anyway.

@Beforerr Beforerr force-pushed the perf/test-inverse-load-time branch from 70c8b41 to 441dcba Compare May 18, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants