Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Speed up Get-GitStatus #319

Merged
merged 1 commit into from
Dec 28, 2016
Merged

Speed up Get-GitStatus #319

merged 1 commit into from
Dec 28, 2016

Conversation

lzybkr
Copy link
Collaborator

@lzybkr lzybkr commented Dec 27, 2016

These changes shouldn't change the core logic of Get-GitStatus,
they are simply avoiding some performance problems in PowerShell.

Briefly, the changes include:

  • Avoid array addition by using List[string]
  • Use the looping capability of the switch statement
  • Use break/continue to avoid testing switch conditions
  • Use a custom implementation of Select -Unique
  • Avoid calling dbg if it would do nothing

I tested these changes in a repo 5000 files staged:
Before: 3.33s
After: 0.15s

These changes shouldn't change the core logic of Get-GitStatus,
they are simply avoiding some performance problems in PowerShell.

Briefly, the changes include:

* Avoid array addition by using List[string]
* Use the looping capability of the switch statement
* Use break/continue to avoid testing switch conditions
* Use a custom implementation of Select -Unique
* Avoid calling dbg if it would do nothing

I tested these changes in a repo 5000 files staged:
Before: 3.33s
After:  0.15s
Copy link
Collaborator

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rkeithhill rkeithhill merged commit 536c57f into dahlbyk:master Dec 28, 2016
@lzybkr lzybkr deleted the status-perf branch December 28, 2016 20:03
$working = New-Object PSObject @(,@($workingPaths | ?{ $_ } | Select -Unique)) |
#
# This collection is used twice, so create the array just once
$filesAdded = $filesAdded.ToArray()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delayed review... I'm curious why $filesAdded is special here? I don't see anything about its usage below that would be problematic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change anything, so I create a single array for the two places this is used. All the others are used just once, so inline was cleaner.

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

Successfully merging this pull request may close these issues.

3 participants