Improve performance of Get-GitHubContent #232
Merged
Conversation
This file contains 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
The call to `ConvertTo-SmarterObject` was adding a significant amount
of time to the execution of Get-GitHubContent.
```powershell
Set-GitHubConfiguration -DisableSmarterObjects:$false
Measure-Command { $c = Get-GitHubContent -OwnerName microsoft -RepositoryName PowerShellForGitHub -Path README.md }
# This averages to be around 1.2 seconds
Set-GitHubConfiguration -DisableSmarterObjects:$true
Measure-Command { $c = Get-GitHubContent -OwnerName microsoft -RepositoryName PowerShellForGitHub -Path README.md }
# This averages to be around 14 seconds
```
The reasoning behind this was that the return of `Invoke-WebRequest`
for this repo's `README.md` was an `[Object[]]` that contained 22,000+
entries (one byte per entry). Piping in 22,000 objects and just writing
each of them out to via `Write-Output` took a _lot_ of time.
The fix here was to skip trying to convert the result to a smarter object
if it wasn't going to be a convertible object in the first place.
This doesn't end up saving much time with the execution of
`Tests/GitHubContents.tests.ps1` because the file being returned
is so small, but this should have some nice user experience improvements
in practical usage.
|
/azp run PowerShellForGitHub-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run PowerShellForGitHub-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
The call to
ConvertTo-SmarterObjectwas adding a significant amount of time to the execution of Get-GitHubContent.The reasoning behind this was that the return of
Invoke-WebRequestfor this repo'sREADME.mdwas an array of [int32] that contained 22,000+ entries (one byte per entry). Because the array was created byConverFrom-Json, each[int32]was also an[object], which meant that it was considered a[PSCustomObject]inside ofConverTo-SmarterObject, which caused it to do a lot of unnecessary work. Piping in 22,000 objects and doing all of that unnecessary work to end up just callingWrite-Outputon the original value took a lot of time.The fix here was to skip trying to convert the result to a smarter object if it wasn't going to be a convertible object in the first place.
I also added in protection directly to
ConverTo-SmarterObjectas well. It made things better than the current runtime, but still twice as slow as not calling it in the first place (average runtime of above command was 3.5 seconds when we allowed it to go into the command and then do nothing).This doesn't end up saving much time with the execution of
Tests/GitHubContents.tests.ps1because the file being returned is so small, but this should have some nice user experience improvements in practical usage.Issues Fixed
None
References
n/a
Checklist
Comment-based help added/updated, including examples.Changes to the manifest file follow the manifest guidance.Relevant usage examples have been added/updated in USAGE.md.If desired, ensure your name is added to our Contributors list