Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

high memory usage #124

Closed
dorianmariecom opened this issue Jan 5, 2022 · 5 comments
Closed

high memory usage #124

dorianmariecom opened this issue Jan 5, 2022 · 5 comments

Comments

@dorianmariecom
Copy link

retained memory by gem
-----------------------------------
   2643413  dead_end-3.1.1

allocated objects by location
-----------------------------------
    252404  /Users/dorianmariefr/.rvm/gems/ruby-3.0.3/gems/dead_end-3.1.1/lib/dead_end/core_ext.rb:19
    118590  /Users/dorianmariefr/.rvm/gems/ruby-3.0.3/gems/dead_end-3.1.1/lib/dead_end/core_ext.rb:30

allocated objects by file
-----------------------------------
    372794  /Users/dorianmariefr/.rvm/gems/ruby-3.0.3/gems/dead_end-3.1.1/lib/dead_end/core_ext.rb

allocated objects by gem
-----------------------------------
    372794  dead_end-3.1.1

and in case it might be useful:

allocated memory by class
-----------------------------------
  30325876  String
  15374176  File
   3715336  Class
   3533088  Hash
   2718504  Array
   2186776  MatchData
    825600  Proc
    630626  Regexp
    596920  Module

this gem is in my bundle because I run the derailed benchmarks

@dorianmariecom
Copy link
Author

basically 40MB it seems like

@dorianmariecom
Copy link
Author

oops forgot to put derailed_benchmarks in development group

@dorianmariecom
Copy link
Author

hmmm even in development it is required by bundle exec derailed bundle:objects

@schneems
Copy link
Collaborator

schneems commented Jan 6, 2022

Memory profiler

That line in dead end isn't doing anything, it's forwarding to your original require call https://github.com/zombocom/dead_end/blob/10232e47fbf4c1372f92b3057cbdc9876f771ca9/lib/dead_end/core_ext.rb#L19. You'll have the same issue if you put bootstrap (or any other gem that aliasesrequire).

Basically whatever file you're loading is actually allocating all that memory. Dead end is getting the blame because it's in the middle.

Dead end monkeypatching

The reason dead end needs that monkeypatch is SyntaxError cannot be monkeypatched. There's a plan to move SyntaxError to be patchable via Ruby core, then we could hook into the exception directly instead of this round-about method which causes confusing results like this sometimes.

Getting require memory with dead_end

If the memory is showing up at boot/require time then it should show up with the other tasks: bundle exec derailed bundle:mem or bundle exec derailed bundle:objects.

Fun fact, derailed uses the same trick to profile require memory (overwriting require) as dead end uses to catch syntax errors today.

@dorianmariecom
Copy link
Author

ok makes sense thanks

schneems added a commit that referenced this issue May 23, 2022
Currently dead_end works by monkey patching require. This causes confusion and problems as other tools are not expecting this. For example zombocom/derailed_benchmarks#204 and #124. This PR utilizes the new SyntaxError#detailed_message as introduced in ruby/ruby#5516 that will be released in Ruby 3.2.

That means that developers using dead_end with Ruby 3.2+ will experience more consistent behavior.


## Limitations

As pointed out in #31 the current version of dead_end only works if the developer requires dead_end and then invokes `require`.

This behavior is still not fixed for Ruby 3.2+ 

```
$ ruby -v
ruby 3.2.0preview1 (2022-04-03 master f801386f0c) [x86_64-darwin20]
$ cat monkeypatch.rb
  SyntaxError.prepend Module.new {
    def detailed_message(highlight: nil, **)
      message = super
      message += "Monkeypatch worked\n"
      message
    end
  }

  # require_relative "bad.rb"
  # Note that i am commenting
  # out the require, but leaving
  # in the monkeypatch
⛄️ 3.2.0 🚀 /tmp
$ cat bad.rb
  def lol_i-am-a-synt^xerror
⛄️ 3.2.0 🚀 /tmp
$ ruby -r./monkeypatch.rb bad.rb
bad.rb:1: syntax error, unexpected '-', expecting ';' or '\n'
  def lol_i-am-a-synt^xerror
```

Additionally we are still not able to handle the case where a program is streamed to ruby and does not exist on disk:

```
$ echo "def foo" | ruby
```

As the SyntaxError does not provide us with the contents of the script.

```
$ echo "def foo" | ruby
-:1: syntax error, unexpected end-of-input
def foo
```
schneems added a commit that referenced this issue May 23, 2022
Currently dead_end works by monkey patching require. This causes confusion and problems as other tools are not expecting this. For example zombocom/derailed_benchmarks#204 and #124. This PR utilizes the new SyntaxError#detailed_message as introduced in ruby/ruby#5516 that will be released in Ruby 3.2.

That means that developers using dead_end with Ruby 3.2+ will experience more consistent behavior.

## Limitations

As pointed out in #31 the current version of dead_end only works if the developer requires dead_end and then invokes `require`.

This behavior is still not fixed for Ruby 3.2+

```
$ ruby -v
ruby 3.2.0preview1 (2022-04-03 master f801386f0c) [x86_64-darwin20]
$ cat monkeypatch.rb
  SyntaxError.prepend Module.new {
    def detailed_message(highlight: nil, **)
      message = super
      message += "Monkeypatch worked\n"
      message
    end
  }

  # require_relative "bad.rb"
  # Note that i am commenting
  # out the require, but leaving
  # in the monkeypatch
⛄️ 3.2.0 🚀 /tmp
$ cat bad.rb
  def lol_i-am-a-synt^xerror
⛄️ 3.2.0 🚀 /tmp
$ ruby -r./monkeypatch.rb bad.rb
bad.rb:1: syntax error, unexpected '-', expecting ';' or '\n'
  def lol_i-am-a-synt^xerror
```

Additionally we are still not able to handle the case where a program is streamed to ruby and does not exist on disk:

```
$ echo "def foo" | ruby
```

As the SyntaxError does not provide us with the contents of the script.

```
$ echo "def foo" | ruby
-:1: syntax error, unexpected end-of-input
def foo
```
schneems added a commit to schneems/ruby that referenced this issue Jul 27, 2022
Adds the `syntax_suggest` syntax error display tool to Ruby through the same mechanism as `error_highlight` and `did_you_mean`. Reference ticket: https://bugs.ruby-lang.org/issues/18159

close ruby#4845

## What is syntax_suggest?

When a syntax error is raised by requiring a file, dead_end will use a combination of indentation and lexing to identify the problem.

> Note: Previously this tool was named `dead_end`. 

## Known issues

- SyntaxSearch's approach of showing syntax errors only works through integration with `require`, `load`, `autoload`, and `require_relative` (since it monkeypatches them to detect syntax errors). It does not work with direct Ruby file invocations ruby/syntax_suggest#31.
  - This causes failure in the test suite (test_expected_backtrace_location_when_inheriting_from_basic_object_and_including_kernel) and confusion when inspecting backtraces if there's a different error when trying to require a file such as measuring memory (ruby/syntax_suggest#124 (comment)).
  - Discussed fix. We previously talked about opening up `SyntaxError` to be monkeypatched in the same way that other gems hook into `NoMethodError`. This is currently not possible and requires development work. When we last talked about it at RubyKaigi Nobu expressed an ability to make such a change.
hsbt pushed a commit to ruby/ruby that referenced this issue Aug 19, 2022
Adds the `syntax_suggest` syntax error display tool to Ruby through the same mechanism as `error_highlight` and `did_you_mean`. Reference ticket: https://bugs.ruby-lang.org/issues/18159

close #4845

## What is syntax_suggest?

When a syntax error is raised by requiring a file, dead_end will use a combination of indentation and lexing to identify the problem.

> Note: Previously this tool was named `dead_end`. 

## Known issues

- SyntaxSearch's approach of showing syntax errors only works through integration with `require`, `load`, `autoload`, and `require_relative` (since it monkeypatches them to detect syntax errors). It does not work with direct Ruby file invocations ruby/syntax_suggest#31.
  - This causes failure in the test suite (test_expected_backtrace_location_when_inheriting_from_basic_object_and_including_kernel) and confusion when inspecting backtraces if there's a different error when trying to require a file such as measuring memory (ruby/syntax_suggest#124 (comment)).
  - Discussed fix. We previously talked about opening up `SyntaxError` to be monkeypatched in the same way that other gems hook into `NoMethodError`. This is currently not possible and requires development work. When we last talked about it at RubyKaigi Nobu expressed an ability to make such a change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants