Skip to content

Suggest require paths on LoadError - #143

Merged
yuki24 merged 1 commit into
masterfrom
require-path-checker
May 9, 2020
Merged

yuki24 merged 1 commit into
masterfrom
require-path-checker

Conversation

@yuki24

@yuki24 yuki24 commented May 3, 2020 •

Copy link
Copy Markdown
Member

In response to https://bugs.ruby-lang.org/issues/16824, I'm adding a "did you mean?" to the LoadError. Here is an example of how it looks like:

 require 'net-http'
 # => LoadError (cannot load such file -- net-http)
 #    Did you mean?  net/http

Performance

Finding suggestions on a large app could negatively impact the performance of the LoadError#message. On a 8-year old Rails app, there are 12676 requirable files:

DidYouMean::RequirePathChecker.requireables.size
# => 12676

And there is a benchmark:

Warming up --------------------------------------
 original (with a /)    25.000  i/100ms
original (without /)     8.000  i/100ms
Calculating -------------------------------------
 original (with a /)    262.268  (± 4.2%) i/s -      2.625k in  10.028352s
original (without /)     74.966  (±17.3%) i/s -    712.000  in  10.015199s

Comparison:
 original (with a /):      262.3 i/s
original (without /):       75.0 i/s - 3.50x  slower

So it would take roughly 13ms for e.g. require 'open_struct' and 3.81ms for e.g. net/htt. The performance difference comes with the fact that DYM uses the tree spell checker (generally more accurate and faster for path names) if possible, but falls back to the traditional spell checker. I don't think this is a bad start.

There is another concern in finding requirable files being a bottleneck due to disk I/O in Dir.glob. There is room for improvement, but for now let's leave it there.

Todos

  • Write up more details
  • Update README and CHANGELOG
  • Add benchmark results

@yuki24 yuki24 self-assigned this May 3, 2020
@yuki24
yuki24 force-pushed the require-path-checker branch 4 times, most recently from 1ac5151 to 875447b Compare May 3, 2020 04:01
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.

1 participant