This repository was archived by the owner on Dec 5, 2024. It is now read-only.
Fix to improve performance of ChangesTreeView #119
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Method | Mean | Error | StdDev | ----------------------------- |---------:|---------:|---------:| OriginalTreeBuilderBenchmark | 65.67 ms | 1.296 ms | 1.730 ms | CurrentTreeBuilderBenchmark | 55.80 ms | 1.102 ms | 1.683 ms |
StanleyGoldman
changed the base branch from
master
to
fixes/changes-view-refactoring
Method | Mean | Error | StdDev | ----------------------------- |---------:|----------:|----------:| OriginalTreeBuilderBenchmark | 61.32 ms | 0.8018 ms | 0.7500 ms | CurrentTreeBuilderBenchmark | 42.24 ms | 0.8295 ms | 1.0786 ms |
Method | Mean | Error | StdDev | ----------------------------- |---------:|----------:|----------:| OriginalTreeBuilderBenchmark | 63.59 ms | 0.9887 ms | 0.9248 ms | CurrentTreeBuilderBenchmark | 24.41 ms | 0.4952 ms | 0.7854 ms |
|
https://gist.github.com/StanleyGoldman/c84e31e174a91e1ff28770feaaf8da29 I've been using this python script to generate data to test with, this is an improvement, but I definitely feel the need for more. |
This was referenced Jul 13, 2017
# Conflicts: # src/GitHub.Api/UI/TreeBuilder.cs
…-performance # Conflicts: # src/GitHub.Api/UI/TreeBuilder.cs
…-performance # Conflicts: # src/GitHub.Api/UI/TreeBuilder.cs
shana
approved these changes
Jul 31, 2017
|
It's an nice improvement, but it doesn't solve the overall performance issues. |
StanleyGoldman
changed the base branch from
fixes/changes-view-refactoring
to
master
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempted #23
performance test used pumps two sequences into
TreeBuilder.BuildTreeRoot():a. The first sequence contains 200
GitStatusEntryobjects.a. The second sequence contains 300
GitStatusEntryobjects.a. There are 10 folders, with 5 files and 5 meta files.
a. Half of the original files can be considered "removed".
a. An additional 10 folders, with 10 files and 10 meta files.
a. The first sequence contains 800
GitStatusEntryobjects.a. The second sequence contains 1200
GitStatusEntryobjects.a. There are 10 folders, with 10 files and 10 meta files.
a. Half of the original files can be considered "removed".
a. An additional 20 folders, with 20 files and 20 meta files.
This branch will let you reproduce the performance test framework:
https://github.com/github-for-unity/Unity/tree/fixes/changes-view-performance-proof
Run the PerformanceTest project un-attached in Release mode.
It will take approx 8-9 minutes to complete.
Overall the PR starts at a third of the time of the current code base, and the amount of time it takes to execute grows bu half of the current code base.
Depends on: