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

degraded performance in 1.6.0 release #490

Closed
codingjoe opened this issue May 13, 2019 · 11 comments · Fixed by #502
Closed

degraded performance in 1.6.0 release #490

codingjoe opened this issue May 13, 2019 · 11 comments · Fixed by #502
Labels
bug Something isn't working
Milestone

Comments

@codingjoe
Copy link

codingjoe commented May 13, 2019

Describe the bug

I have seen a performance decrease in version 1.6.0. I a member of FussyFox and we run thousands of bandit checks a day. I see an increase in timeouts (200s limit) since I upgraded the stack to the 1.6.0 release.

My best guess, it's this commit 7c4b9fa
I will further investigate the issue.

Screen Shot 2019-05-14 at 17 32 02
The 200-second cap is because the sub process times out. The execution time would be even longer.

codingjoe referenced this issue May 13, 2019
This commit adds a new command line argument to hide output
when the result of the scan is passing with no warning/errors.

The new option is -q or --quiet. It also allows --silent to be
consistent with the GNU standard on CLI options [1].

[1] http://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table

Signed-off-by: Eric Brown <browne@vmware.com>

Fixes Issue #384
@codingjoe
Copy link
Author

I believe those two lines cause the performance decrease:

issues = manager.get_issue_list(sev_level, conf_level)
if len(issues) or not manager.quiet:

I will further investigate.

codingjoe added a commit to codingjoe/bandit that referenced this issue May 13, 2019
The lines were introduced in 7c4b9fa
and have two effects. First they cause `get_issue_list` to run twice and before
the user receives feedback that bandit started running. Secondly it does not
display any output if no issues are found, which is an unintended behavior change.
@codingjoe
Copy link
Author

@ericwb I am cc-ing you here too, since you released 1.6.0. Maybe you have another hint why the performance decreased so drastically. You can see in figure 1 how big the impact is, after the update.

@ericwb ericwb added this to the Release 1.6.1 milestone May 14, 2019
@ericwb ericwb added the bug Something isn't working label May 14, 2019
@ehooo
Copy link
Contributor

ehooo commented May 29, 2019

Hi @codingjoe I just try to find the delay on the code and I cannot find any on the Text formatter.
are you sure that it is not related to issue #438 ?

Could you share more information in order to find the root cause?
Could you tell us the parameters using to run the report?

When I run the line_profile using the example directory it not take delay.

Total time: 0.019999 s
File: ~/code/bandit/bandit/formatters/text.py
...
   151                                           
   152         1          0.0      0.0      0.0          bits.append("\nTest results:")
   153         1      18184.0  18184.0     90.9          bits.append(get_results(manager, sev_level, conf_level, lines))
   154         1          2.0      2.0      0.0          bits.append("\nCode scanned:")
...

@codingjoe
Copy link
Author

@ehooo I am not so sure, it's the same problem, since the spike in run time only happens since version 1.6.0 not 1.5.x

@ehooo
Copy link
Contributor

ehooo commented Jun 10, 2019

@codingjoe could you share more context to try to reproduce the issue?
Some project affected? the args used to run the report? the report format?, ...

@codingjoe
Copy link
Author

@ehooo no specific project, this seems to have affected all kinds of projects.
Bandit is invoked as follows https://github.com/FussyFox/bandit/blob/39772b3e6528ab1f062f7b4190a9993d5937e78d/main.py#L12-L15

@bittner
Copy link
Contributor

bittner commented Jun 12, 2019

I am affected by this issue, too. It seem not to be a general performance issue, but really a bug that makes Bandit collect a large amount of files to inspect.

  • When I run Bandit 1.5.1 over my project it takes a fraction of a second.
  • When I run Bandit 1.6.0 over my project it counts something to almost 3000 and the CPU load goes up.

Example:

$ tox -e bandit                                                                                                                                                                                                                                                           
bandit installed: bandit==1.6.0,gitdb2==2.0.5,GitPython==2.1.11,pbr==5.2.1,PyYAML==5.1.1,six==1.12.0,smmap2==2.0.5,stevedore==1.30.1                                                                                                                                            
bandit run-test-pre: PYTHONHASHSEED='1925481915'                                                                                                                                                                                                                                
bandit runtests: commands[0] | bandit -r --ini tox.ini                                                                                                                                                                                                                          
[main]  INFO    Using ini file for excluded paths                                                                                                                                                                                                                               
[main]  INFO    Using ini file for selected targets                                                                                                                                                                                                                             
[main]  INFO    profile include tests: None                                                                                                                                                                                                                                     
[main]  INFO    profile exclude tests: None                                                                                                                                                                                                                                     
[main]  INFO    cli include tests: None                                                                                                                                                                                                                                         
[main]  INFO    cli exclude tests: None                                                                                                                                                                                                                                         
[main]  INFO    running on Python 3.6.8                                                                                                                                                                                                                                         
2946 [0.. 50.. 100.. 150.. 200.. 250.. 300.. 350.. ^C

Could it be that for some reason the excluded paths option has become defunct?

ericwb pushed a commit that referenced this issue Jun 22, 2019
* Fix #490 -- Fix performance issue introduced in 1.6.0

The lines were introduced in 7c4b9fa
and have two effects. First they cause `get_issue_list` to run twice and before
the user receives feedback that bandit started running. Secondly it does not
display any output if no issues are found, which is an unintended behavior change.

* add namespaces for parent attributes

* pylint formatting changes

* made bandit_parent a private attr

* temporary fix; perf issue only on quiet

* update perf issue
@ericwb ericwb modified the milestones: Release 1.6.1, Release 1.6.2 Jun 22, 2019
@ehooo
Copy link
Contributor

ehooo commented Jul 4, 2019

Hi again.

I was working trying to find the root cause for that issue, but i think is affected just for some kind of code.
I mean is run bandit with trying to find the bottleneck.

The following code was used:

pip install bandit==1.5.1
git clone --branch 1.5.0 --single-branch https://github.com/PyCQA/bandit.git ../bandit
python analyse_bandit.py -r ../bandit/examples/

pip install bandit==1.6.0
python analyse_bandit.py -r ../bandit/examples/

pip install bandit==1.6.1
python analyse_bandit.py -r ../bandit/examples/

The results:
1.5.1py27 102.7241
1.6.0py27 97.6364
1.6.1py27 95.4089
1.5.1py3.7 4.2379
1.6.0py37 4.5011
1.6.1py37 4.7639

On python 2.7 we could see the bottleneck on "fnmatchcase"
1 5 1py27
1 6 1py27

But on 3.7 the only i could find the functions "isfile", "get_module_qualname..", and "_execute_ast_visit.."
1 5 1py37
1 6 0py37
1 6 1py37

But i not sure that it is the root case.

@bittner i think you issue is related with #488

Could someone give us some feedback to know if this case is solved on 1.6.1 or 1.6.2 version?

@codingjoe
Copy link
Author

codingjoe commented Jul 5, 2019

Thanks @ehooo for the in depth profiling. I upgraded @FussyFox two days ago from 1.5.1 to 1.6.2 and do NOT see an increase in real user time across a couple thousand executions across various code bases.

Screenshot 2019-07-05 at 16 56 27

I guess case closed :)

@codingjoe
Copy link
Author

Nope, I was immaturely excited :/
FussyFox/fussyfox.github.io#4 (comment)
There still seems to be an issue with version 1.6.x
I am trying to figure out if there is another problem showing up in the logs. I do see a slightly elevated error rate since the new release.

@bittner
Copy link
Contributor

bittner commented Oct 10, 2019

Why is this issue still closed? Shouldn't we reopen it?

I'm still pinning Bandit to <1.6, because bandit -r . scans .tox (and presumably similar folders such as .git) since 1.6.0 even though I use the --exclude .tox, ... option. This obviously takes significantly longer to complete as that folder typically contains several virtualenvs with a load full of Python packages downloaded from PyPI.

bittner added a commit to behave/behave-django that referenced this issue Jan 10, 2022
Bandit UX is seriously broken, only <1.6 works predictably.

Exclude/ignore of files is currently broken in Bandit:
- PyCQA/bandit#693
- PyCQA/bandit#490
- PyCQA/bandit#438 (comment)

Reading settings from configuration files is broken:
- PyCQA/bandit#753
- PyCQA/bandit#595

Reading from pyproject.toml not yet functional:
- Must install "toml" package and use "-c pyproject.toml".
- PyCQA/bandit#758

INI file configuration and CLI usage is unclear:
- PyCQA/bandit#603
- PyCQA/bandit#467
- PyCQA/bandit#396
bittner added a commit to behave/behave-django that referenced this issue Jan 10, 2022
Bandit UX is seriously broken, only <1.6 works predictably.

Exclude/ignore of files is currently broken in Bandit:
- PyCQA/bandit#693
- PyCQA/bandit#490
- PyCQA/bandit#438 (comment)

Reading settings from configuration files is broken:
- PyCQA/bandit#753
- PyCQA/bandit#595

Reading from pyproject.toml not yet functional:
Must install "toml" package and use "-c pyproject.toml".
- PyCQA/bandit#758

INI file configuration and CLI usage is unclear:
- PyCQA/bandit#603
- PyCQA/bandit#467
- PyCQA/bandit#396
bittner added a commit to behave/behave-django that referenced this issue Jan 10, 2022
Bandit UX is seriously broken, only <1.6 works predictably.

Exclude/ignore of files is currently broken in Bandit:
- PyCQA/bandit#693
- PyCQA/bandit#490
- PyCQA/bandit#438 (comment)

Reading settings from configuration files is broken:
- PyCQA/bandit#753
- PyCQA/bandit#595

Reading from pyproject.toml not yet functional:
Must install "toml" package and use "-c pyproject.toml".
- PyCQA/bandit#758

INI file configuration and CLI usage is unclear:
- PyCQA/bandit#603
- PyCQA/bandit#467
- PyCQA/bandit#396
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants