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

Plugin performs slowly on large lcov.info files #185

Closed
cooldome opened this issue Nov 6, 2018 · 16 comments
Closed

Plugin performs slowly on large lcov.info files #185

cooldome opened this issue Nov 6, 2018 · 16 comments
Labels
Milestone

Comments

@cooldome
Copy link

cooldome commented Nov 6, 2018

Describe the bug
It takes 55 seconds on the top spec workstation to display green/red coverage ruler for small project with a single lcov.info file. lcov.info contains per line stats for 45 files in the project, total size is 42 kb.
It actually makes plugin unusable.
I have spend some time profiling and looks like all time spent in lcov-parse npm plugin (https://github.com/davglass/lcov-parse). Checking its code you can clearly see why https://github.com/davglass/lcov-parse/blob/master/lib/index.js: file streams not used, every line in the file gets copied N times where N is number lines. Algorithm is O(N^2) complexity for no real reason.

I suggest not to use this plugin or try submitting PR to fix it. Unfortunately, I don't program in js to do it myself.

Desktop (please complete the following information):

  • OS: windows
  • Extension Version [e.g. 2.2.0]
@ryanluker
Copy link
Owner

@cooldome thanks for submitting an issue. Can you try uninstalling the extension and install the one mentioned here #182 (comment) ? If that doesnt help any then I can look into cleaning this performance up in the 2.3.0 version.

@SimonKagstrom
Copy link

I also experience this issue (over a minute before display), but with cobertura XML input. My input file is 128K big, and contains coverage info for 20 files. My project is fairly large though, with around 8000 files, but I think it should be fairly quick since the coverage is only collected for a very small part of it.

I've tried the 2.2.2 version mentioned in #182, but it doesn't help for me.

@cooldome
Copy link
Author

cooldome commented Nov 7, 2018

Hi Ryanluker,

I have tried the version you have proposed, the time went down from 55 seconds to 47 seconds, but it is still unusable at this performance. All my comments still apply, most time is spent in lcov-parse. The same behaviour: one CPU core is loaded 100% for 45 seconds at least

@ryanluker
Copy link
Owner

@cooldome @SimonKagstrom @mtycholaz I have a new version of 2.2.2 attached below that has some performance improvements hopefully. Can you guys test it and see how it looks?

vscode-coverage-gutters-2.2.2.zip

ryanluker added a commit that referenced this issue Nov 12, 2018
ryanluker added a commit that referenced this issue Nov 12, 2018
ryanluker added a commit that referenced this issue Nov 12, 2018
ryanluker added a commit that referenced this issue Nov 12, 2018
ryanluker added a commit that referenced this issue Nov 12, 2018
Add a new check coverage helper
@SimonKagstrom
Copy link

I'm not sure, there might have been an improvement with the latest version, but it still takes around a minute for me (with only cobertura files generated).

@cooldome
Copy link
Author

Hi, I have tried new vscode-coverage-gutters-2.2.2.zip and I haven't noticed significant improvements in the performance. I did profiling again and noticed that CPU is a lot less loaded now but time spent still a lot.

I managed to trace down and fix the issue by changing one line in Coverageparser.js:

if (!path.startsWith(".") && path.startsWith("/")) {
  return path;
}

replaced with

if (!path.startsWith(".") && (path.startsWith("/") || (path[1] == ':' && path[2] == '\\'))) {
  return path;
}

It helps with recognising the absolute paths for windows.
With new vscode-coverage-gutters-2.2.2 and this fix the coverage is displayed almost instantly.

@ryanluker
Copy link
Owner

ryanluker commented Nov 18, 2018

@cooldome good find! This will definitely help windows users that have their coverage outputted with absolute paths. I did find though that many users (dart for example on googles flutter framework) did have relative file paths and this would still be an issue for them but there is no reason to punish windows users that do have proper pathing 😆 .

@ryanluker ryanluker added this to the 2.3.0 milestone Nov 18, 2018
@SimonKagstrom
Copy link

I'm on UNIX (OSX and Linux), so I suppose this won't help in my case. Not sure how to do profiling like @cooldome did.

@krishnakumar3044
Copy link

I see the same issue of very "slow"(more than a minute) with moderate project.

my cov.xml file size is below


-rw-rw-r-- 1 user2 user2 996297 Nov 27 23:13 cov.xml

@ryanluker
Copy link
Owner

@krishnakumar3044 @SimonKagstrom do your coverage files have absolute paths or relative? In the newer versions of the extension (2.2.0 and up) it tries to convert the relative to absolute to make it easier later to match a coverage to a file that is opened in the editor (I chose this over resolving relative -> absolute every file open as that would produce a worse user experience in my mind).

ryanluker added a commit that referenced this issue Dec 2, 2018
@ryanluker
Copy link
Owner

@krishnakumar3044 @SimonKagstrom you can also try the latest pre-release version below to see if that helps.
vscode-coverage-gutters-2.3.0.zip

@SimonKagstrom
Copy link

@ryanluker Unfortunately, I see no particular improvement with 2.3.0, it's still in the order of minutes here.

@RoyalBingBong
Copy link

RoyalBingBong commented Dec 6, 2018

I feel like this also impacts the general performance of Code. E.g IntelliSense doesn't load until I force it to load via Ctrl+Space. Same for snippets auto completion.

It's all good again when the coverage is finally displayed.

Edit: I'm using 2.2.2 with Code 1.29.1 on Ubuntu 16.04.

@ryanluker
Copy link
Owner

ryanluker commented Dec 9, 2018

@RoyalBingBong the extension runs in an async style while loading the files to find their absolute path but yes this would impact the other vscode systems while this caching occurs.

After thinking about this some more, it might have been not the best idea to put the relative -> absolute caching at the very beginning of the extensions lifecycle (IE forcing the cache to have absolutes before anything else could happen) as this has many performance consequences for larger projects.

I am going to make some adjustments to the caching flow to instead convert relative -> absolute paths only when a file is opened and only for that one file. This may solve some of the issues outlined here.

@ryanluker
Copy link
Owner

ryanluker commented Dec 9, 2018

@RoyalBingBong @SimonKagstrom @krishnakumar3044 can you guys try the pre-release below but also paste the output from the gutters channel?
vscode-coverage-gutters-2.3.0-prerelease.zip

screenshot from 2018-12-09 11-45-52

@ryanluker
Copy link
Owner

Going to close this ticket as it is getting long in the tooth and @cooldome 's originally issue has been fixed with the improvements to the absolute path detector. I am more then happy to continue adding performance improvements if people wish to open new tickets specific to their situation though 😃 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants