Improve completion speed in large applications by dgutov · Pull Request #1588 · pry/pry

dgutov · 2016-12-12T01:43:01Z

This is another attempt to improve completion speed (e.g. #1540). It's a non-caching solution which improved full scan completion speed in our production app from 5s to 0.16s.

Even if we end up adding cache, I'd rather it didn't take long to populate.

Relates to pry#1540.

Traversing via constant lookup is constant-time, as opposed to O(number of defined modules). And it doesn't use Module#name. So this is fast.

Like Argon2::Password in argon2 < v1.1.1.

irb_exit is defined on IRB's singleton class.

uniq! uses a hash table anyway. But now sort! needs to deal with a shorter list.

printercu · 2016-12-12T08:27:09Z

lib/pry/input_completer.rb

-            end
-            next if name != "IRB::Context" and
-            /^(IRB|SLex|RubyLex|RubyToken)/ =~ name
+            next if to_ignore.include?(m) rescue true


3.times { |i| next if asd rescue true; puts i } prints 0,1,2. Is it expected?

I don't understand why rescue is here, while ignored_modules returns array all the time.

ignored_modules returns a set, but that's not why this is here, see the message in 353a42b.

The previous versions of Argon2::Password overrode self.hash. And Set#include? calls #hash on its argument, which in this case raised ArgumentError. So this is to deal with misbehaving third-party code.

Not sure if we can do better; using a set is measurably faster here than using an array.

Got it. If such modules should be skipped, should i change it to next if (to_ignore.include?(m) rescue true)?

That would be fine, too.

Actually, thanks for the catch.

@dgutov

By @dgutov from pry#1588

printercu · 2016-12-12T09:22:00Z

Copied this one on top of #1583, benchmarks at #1583 (comment)

dgutov · 2016-12-13T01:00:59Z

@banister @mvz Do you think we can get this into 0.11.0, or is it too late?

dgutov · 2016-12-15T10:53:31Z

@rf- @R-obert Same question.

benoittgt · 2017-04-07T13:27:34Z

I would love to see this PR merge. 💕

0x1eef · 2017-04-07T18:14:41Z

LGTM.

printercu · 2017-04-08T07:22:39Z

@R-obert #1583 brings cache to methods list, which is much faster. There is benchmark

0x1eef · 2017-04-08T08:22:38Z

yeah but the implementation in this PR is more simple and still includes a significant speed up (5s to 0.16s). we could add the cache in a follow up pull request. it would probably be easier to review then as well. if i understood correctly, with a cache we gain 100ms speed improvement?

dgutov · 2017-04-08T19:48:14Z

@R-obert Thanks for taking a look. Anything I can do to speed this along?

What are the odds of getting this into a release soon?

0x1eef · 2017-04-08T23:30:22Z

i'm not sure when the next release will be, but i think this is good to merge. let's just wait and see if anyone else has some input.

dgutov · 2017-04-08T23:36:51Z

@R-obert So 0.11 is a definite no? I could rebase onto v0.11.0.pre, if that might be considered appropriate.

Admittedly, I don't know the reasons for this long pre-release cycle.

0x1eef · 2017-04-08T23:55:23Z

@banister what do you think? can we just release master?

0x1eef · 2017-05-27T14:40:55Z

hey @dgutov - what's the best way to test the performance improvement? i tried this locally in a work app, and i didn't notice any difference with or without your patch.

dgutov · 2017-05-27T15:38:57Z

@R-obert Please see my comment in #1540 (comment).

dgutov · 2017-05-27T15:41:30Z

That's the benchmark. With the patch, you should see entirely different (better) numbers.

0x1eef · 2017-05-27T15:45:48Z

thanks! i can confirm "aaa".<TAB> is much more responsive with your patch. there's a noticeable delay without the patch.

0x1eef · 2017-05-27T15:53:54Z

@dgutov thanks again.

dgutov · 2017-05-27T16:07:05Z

Woohoo!

pkgsrc change: add support for pkg_alternatives ### HEAD #### Features * Add Pry::Testable, an improved modular replacement for PryTestHelpers. **breaking change**. See pull request [#1679](pry/pry#1679). * Add a new category module: "Pry::Platform". Loosely related to #1668 below. See pull request [#1670](pry/pry#1670) * Add `mac_osx?` and `linux?` utility functions to Pry::Helpers::BaseHelpers. See pull request [#1668](pry/pry#1668). * Add utility functions for drawing colorised text on a colorised background. See pull request [#1673](pry/pry#1673). #### Bug fixes * Fix a case of infinite recursion in `Pry::Method::WeirdMethodLocator#find_method_in_superclass` that users of the [Hanami](http://hanamirb.org/) web framework experienced and reported since 2015. See pull request [#1639](pry/pry#1689). * Fix a bug where Method objects were not returned for setters inherited from a default (Pry::Config::Default). Eg, this is no longer an error: pry(main)> d = Pry::Config.from_hash({}, Pry::Config::Default.new) pry(main)> d.method(:exception_whitelist=) # Error See pull request [#1688](pry/pry#1688). * Do not capture unused Proc objects in Text helper methods `no_color` and `no_paging`, for performance reasons. Improve the documentation of both methods. See pull request [#1691](pry/pry#1691). * Fix `String#pp` output color. See pull request [#1674](pry/pry#1674). ### 0.11.0 * Add alias 'whereami[?!]+' for 'whereami' command. ([#1597](pry/pry#1597)) * Improve Ruby 2.4 support ([#1611](pry/pry#1611)): * Deprecated constants are hidden from `ls` output by default, use the `-d` switch to see them. * Fix warnings that originate in Pry while using the repl. * Improve completion speed in large applications. ([#1588](pry/pry#1588)) * Pry::ColorPrinter.pp: add `newline` argument and pass it on to PP. ([#1603](pry/pry#1603)) * Use `less` or system pager pager on MS Windows if it is available. ([#1512](pry/pry#1512)) * Add `Pry.configure` as an alternative to the current way of changing configuration options in `.pryrc` files. ([#1502](pry/pry#1502)) * Add `Pry::Config::Behavior#eager_load!` to add a possible workaround for issues like ([#1501](pry/pry#1501)) * Remove Slop as a runtime dependency by vendoring v3.4 as Pry::Slop. People can depend on Slop v4 and Pry at the same time without running into version conflicts. ([#1497](pry/pry#1497)) * Fix auto-indentation of code that uses a single-line rescue ([#1450](pry/pry#1450)) * Remove "Pry::Config#refresh", please use "Pry::Config#clear" instead. * Defining a method called "ls" no longer breaks the "ls" command ([#1407](pry/pry#1407)) * Don't raise when directory permissions don't allow file expansion ([#1432](pry/pry#1432)) * Syntax highlight <tt> tags in documentation output. * Add support for BasicObject subclasses who implement their own #inspect (#1341) * Fix 'include RSpec::Matchers' at the top-level (#1277) * Add 'gem-readme' command, prints the README file bundled with a rubygem * Add 'gem-search' command, searches for a gem with the rubygems.org HTTP API * Fixed bug in the `cat` command where it was impossible to use line numbers with files ([#1349](pry/pry#1349)) * Fixed uncaught Errno::EOPNOTSUPP exception when $stdout is a socket ([#1352](pry/pry#1352)) * Display a warning when you cd'ed inside a C object and executed 'show-source' without arguments ([#691](pry/pry#691)) * Make the stagger_output method more reliable by reusing possibly available Pry instance ([#1364](pry/pry#1364)) * Make the 'gem-install' message less confusing by removing backticks ([#1350](pry/pry#1350)) * Fixed error when Pry was trying to load incompatible versions of plugins ([#1312](pry/pry#1312)) * Fixed bug when `hist --clear` led to ArgumentError ([#1340](pry/pry#1340)) * Fixed the "uninitialized constant Pry::ObjectPath::StringScanner" exception during autocomplete ([#1330](pry/pry#1330)) * Secured usage of colours with special characters (RL_PROMPT_START_IGNORE and RL_PROMPT_END_IGNORE) in Pry::Helpers::Text ([#493](pry/pry#493 (comment))) * Fixed regression with `pry -e` when it messes the terminal ([#1387](pry/pry#1387)) * Fixed regression with space prefixes of expressions ([#1369](pry/pry#1369)) * Introduced the new way to define hooks for commands (with `Pry.hooks.add_hook("{before,after}_commandName")`). The old way is deprecated, but still supported (with `Pry.commands.{before,after}_command`) ([#651](pry/pry#651)) * Removed old API's using `Pry::Hooks.from_hash` altogether * Removed hints on Foreman support (see [this](ddollar/foreman#536)) * Fixed support for the tee command ([#1334](pry/pry#1334)) * Implemented support for CDPATH for ShellCommand ([#1433](pry/pry#1433), [#1434](pry/pry#1434)) * `Pry::CLI.parse_options` does not start Pry anymore ([#1393](pry/pry#1393)) * The gem uses CPU-less platforms for Windows now ([#1410](pry/pry#1410)) * Add `Pry::Config::Memoization` to make it easier to implement your own `Pry::Config::Default` class.([#1503](pry/pry#1503)) * Lazy load the config defaults for `Pry.config.history` and `Pry.config.gist`.

dgutov added 6 commits December 12, 2016 02:02

Speed up completion by caching the ignored modules

6ab8c95

Relates to pry#1540.

Generate the ignored modules list a different way

cbb148f

Traversing via constant lookup is constant-time, as opposed to O(number of defined modules). And it doesn't use Module#name. So this is fast.

Compute them just one, of course

26e330f

Skip modules that mess with Class#hash

353a42b

Like Argon2::Password in argon2 < v1.1.1.

Fix a typo

8c40837

Use a better method in the blacklisting test

6460ddc

irb_exit is defined on IRB's singleton class.

dgutov mentioned this pull request Dec 12, 2016

Cache all available methods list for expression completion #1583

Closed

Move uniq! before sort!

67a497f

uniq! uses a hash table anyway. But now sort! needs to deal with a shorter list.

printercu reviewed Dec 12, 2016

View reviewed changes

printercu added a commit to printercu/pry that referenced this pull request Dec 12, 2016

Speed up completion by caching the ignored modules

78dd1d8

By @dgutov from pry#1588

Really ignore old Argon2::Password and the like in the methods scan

6a29ed8

Work around an unexpected success under JRuby

707a6e7

dgutov mentioned this pull request Dec 14, 2016

Pry *rails* buffer company not stopping completion after dot dgutov/robe#96

Closed

mvz mentioned this pull request May 13, 2017

New Release? #1448

Closed

0x1eef merged commit 2508d68 into pry:master May 27, 2017

dgutov deleted the completion_speed branch May 27, 2017 16:06

andrehjr mentioned this pull request Apr 23, 2022

TAB autocomplete is super-slow #1540

Closed

dgutov mentioned this pull request Jul 15, 2022

CPU spikes during rails process buffer completion dgutov/robe#139

Closed

Conversation

dgutov commented Dec 12, 2016

Uh oh!

printercu Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

printercu Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

dgutov Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

printercu Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

dgutov Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

dgutov Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

printercu commented Dec 12, 2016

Uh oh!

dgutov commented Dec 13, 2016

Uh oh!

dgutov commented Dec 15, 2016

Uh oh!

benoittgt commented Apr 7, 2017

Uh oh!

0x1eef commented Apr 7, 2017

Uh oh!

printercu commented Apr 8, 2017

Uh oh!

0x1eef commented Apr 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgutov commented Apr 8, 2017

Uh oh!

0x1eef commented Apr 8, 2017

Uh oh!

dgutov commented Apr 8, 2017

Uh oh!

0x1eef commented Apr 8, 2017

Uh oh!

0x1eef commented May 27, 2017

Uh oh!

dgutov commented May 27, 2017

Uh oh!

dgutov commented May 27, 2017

Uh oh!

0x1eef commented May 27, 2017

Uh oh!

0x1eef commented May 27, 2017

Uh oh!

dgutov commented May 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

0x1eef commented Apr 8, 2017 •

edited

Loading