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

Cache the feature scan results #503

Merged
merged 1 commit into from
Jan 28, 2018

Conversation

tlecomte
Copy link

On a solution with about 6000 test scenarios, Pickles.exe takes about 16 seconds on my machine to process.
This is with the following options:
Pickles.exe -f xxx -o xxx --trfmt=nunit3 --enableComments=false -df=json -lr=xxx

Profiling shows that a large part of this time is spent scanning the NUnit results. More precisely, scanning by feature name is done repeatedly on the xml results.
In this commit, the time is reduced to 5 seconds by scanning the xml once and storing the results in a C# lookup, which is optimized for fast-access by key.

Unit tests pass.
The resulting json files are identical (except "GeneratedOn" datetime, obviously).

@dirkrombauts
Copy link
Member

This is a very valuable contribution - a much-needed optimization. Many thanks!

The build fails, sadly ... could you try to run the build.bat batch file from the command line on your development machine and see if that runs through?

@tlecomte
Copy link
Author

Thanks for writing and maintaining Pickles, it's a great tool. This PR is a modest contribution to improve performance on large solutions. This one brings a 3-fold improvement wth no logic changes.

If it turns out to be an appropriate contribution, I have a few other changes to avoid re-creating some objects to bring some more improvement.

"./build" works fine on my machine. The Appveyor failures is strange ("SocketException (0x80004005): An existing connection was forcibly closed by the remote host"), so I've made a dummy change to retry it.

@tlecomte
Copy link
Author

The build succeeded 😄

@dirkrombauts
Copy link
Member

Great, looks simple and if the unit tests pass I trust that it does its job :-)

Could you make an entry in the change log? In changelog.md under Unreleased -> Fixed. Update your repo first because I merged another PR that also contains a change to the changelog.md

@dirkrombauts
Copy link
Member

I will welcome any and all PRs you may send my way!

@tlecomte
Copy link
Author

I've updated the Changelog. Build is still green.

On a solution with about 6000 test scenarios, Pickles.exe takes about 16 seconds on my machine to process.
This is with the following options:
Pickles.exe -f xxx -o xxx --trfmt=nunit3 --enableComments=false -df=json -lr=xxx

Profiling shows that a large part of this time is spent scanning the NUnit results. More precisely, scanning by feature name is done repeatedly on the xml results.
In this commit, the time is reduced to 5 seconds by scanning the xml once and storing the results in a C# lookup, which is optimized for fast-access by key.

Unit tests pass.
The resulting json files are identical (except "GeneratedOn" datetime, obviously).
@dirkrombauts dirkrombauts merged commit ae88a30 into picklesdoc:develop Jan 28, 2018
@dirkrombauts
Copy link
Member

Thanks for your contribution!

@tlecomte
Copy link
Author

Thanks 😄

@dirkrombauts
Copy link
Member

Released in version 2.18.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

Successfully merging this pull request may close these issues.

2 participants