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

Poor performance processing repeated conditional reassignments #1558

Open
amarshall opened this issue Jan 27, 2021 · 2 comments
Open

Poor performance processing repeated conditional reassignments #1558

amarshall opened this issue Jan 27, 2021 · 2 comments

Comments

@amarshall
Copy link

amarshall commented Jan 27, 2021

Background

Brakeman version: 5.0.0
Rails version: 5.2.4.4
Ruby version: 2.7.1

Link to Rails application code: N/A (closed source)

Hanging or Slowness

Below code example was added as ./app/whatever/test.rb in a Rails application. The code is based off real code exhibiting this issue but names are obviously obfuscated and I’ve tried to reduce to a minimal example for reproduction.

Processing times (“calls” means the number of x = fooN(x) if bar? lines):

  • With 30 calls: unknown, user-terminated after 60 s
  • With 20 calls: ~ 10 s
  • With 10 calls: ~ 2.6 s

With terminating with --debug, the top of the stacktrace is:

brakeman-5.0.0/lib/ruby_parser/bm_sexp.rb:32:in `block in deep_clone'
brakeman-5.0.0/lib/ruby_parser/bm_sexp.rb:30:in `each'
brakeman-5.0.0/lib/ruby_parser/bm_sexp.rb:30:in `deep_clone'
… above three lines repeat 10+ times
brakeman-5.0.0/lib/brakeman/processors/alias_processor.rb:86:in `replace'
brakeman-5.0.0/lib/brakeman/processors/alias_processor.rb:75:in `process_default'
brakeman-5.0.0/lib/ruby_parser/bm_sexp_processor.rb:78:in `block in process'
brakeman-5.0.0/lib/ruby_parser/bm_sexp_processor.rb:113:in `in_context'
brakeman-5.0.0/lib/ruby_parser/bm_sexp_processor.rb:72:in `process'
brakeman-5.0.0/lib/brakeman/processors/alias_processor.rb:62:in `block in process_default'
…

Code example:

class BrakemanTest
  def f
    x = foo001(x) if bar?
    x = foo002(x) if bar?
    x = foo003(x) if bar?
    x = foo004(x) if bar?
    x = foo005(x) if bar?
    x = foo006(x) if bar?
    x = foo007(x) if bar?
    x = foo008(x) if bar?
    x = foo009(x) if bar?
    x = foo010(x) if bar?
    x = foo011(x) if bar?
    x = foo012(x) if bar?
    x = foo013(x) if bar?
    x = foo014(x) if bar?
    x = foo015(x) if bar?
    x = foo016(x) if bar?
    x = foo017(x) if bar?
    x = foo018(x) if bar?
    x = foo019(x) if bar?
    x = foo020(x) if bar?
    x = foo021(x) if bar?
    x = foo022(x) if bar?
    x = foo023(x) if bar?
    x = foo024(x) if bar?
    x = foo025(x) if bar?
    x = foo026(x) if bar?
    x = foo027(x) if bar?
    x = foo028(x) if bar?
    x = foo029(x) if bar?
    x = foo030(x) if bar?
    x
  end
end
@presidentbeef
Copy link
Owner

Hi @amarshall - thank you for reporting this and for the great repro test case!

This is definitely a known difficult case for Brakeman, but it should be limited in how much time it takes. I will take a look!

It's not a regression in 5.0, though. Likely what happened is that file in your application was not being scanned in 4.10.1, but now it is in 5.0.

@amarshall amarshall changed the title Severe performance degredation in v5.0.0 with repeated conditional reassignment Poor performance processing repeated conditional reassignments Feb 9, 2021
@amarshall
Copy link
Author

Ah you’re correct, I see that in 4.10.1 the files are not listed as actually being processed when running with --debug. We had presumed that these files would have being scanned before as well, which was obviously incorrect (and I see that the release notes for 5.0.0 indicate more files would be scanned). I’ve updated the issue to reflect that as, while it’s still an issue, it is not clearly related to 5.0.0.

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

No branches or pull requests

2 participants