Skip to content

Style: Remove space after new keyword in implicitly typed array allocation#8505

Merged
iSazonov merged 1 commit intoPowerShell:masterfrom
daxian-dbw:spaceInNew
Dec 21, 2018
Merged

Style: Remove space after new keyword in implicitly typed array allocation#8505
iSazonov merged 1 commit intoPowerShell:masterfrom
daxian-dbw:spaceInNew

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Dec 21, 2018 •
edited
Loading

PR Summary

Remove space after new keyword in implicitly typed array allocation.
654 issues are fixed.

PR Checklist

@daxian-dbw daxian-dbw requested a review from iSazonov December 21, 2018 02:06
@daxian-dbw daxian-dbw requested a review from BrucePay as a code owner December 21, 2018 02:06
@daxian-dbw daxian-dbw removed the request for review from BrucePay December 21, 2018 02:08
@daxian-dbw daxian-dbw added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 21, 2018
iSazonov
iSazonov approved these changes Dec 21, 2018
View reviewed changes
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

@daxian-dbw Recently Jason noted that it makes sense to merge Types_Ps1Xml.cs and TypesV3_Ps1Xml.cs (perhaps some files from FormatAndOutput\DefaultFormatters too). I looked this but there are difficulties for me. The merging may speed up startup.

@iSazonov iSazonov self-assigned this Dec 21, 2018
@iSazonov iSazonov merged commit e999e1d into PowerShell:master Dec 21, 2018
@daxian-dbw daxian-dbw deleted the spaceInNew branch December 21, 2018 18:08
@daxian-dbw
Copy link
Member Author

daxian-dbw commented Dec 21, 2018 •
edited
Loading

@iSazonov What do you mean by merging them? Having a single .cs file that contains the type definitions for both? I don't think that alone will affect the perf, so there should be more that Jason wanted.
I did looked at the CPU time trace with the IL assemblies when working on the startup perf, and the huge yield methods in those 2 files does take an outstanding amount of the CPU time, over 2%. But the data is from IL assemblies and thus I'm not sure if it's really an issue with crossgen'ed assemblies. I couldn't generate PDBs for crossgen'ed assemblies.

I haven't contact .NET Core team on the crossgen related issues/questions. I moved to something with higher priority.

@iSazonov
Copy link
Collaborator

iSazonov commented Dec 21, 2018

Yes, single file. Jason simply mentioned that now there is no reason to have different files. Thanks for clarification about perf.

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Dec 22, 2018

I will get back to those huge yield state-machine methods when switching back to the CPU trace analysis work.

@iSazonov
Copy link
Collaborator

iSazonov commented Dec 22, 2018

Perhaps we could get more win by investigating to Jason's PSMore #7857. I guess that definitely yes at startup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov iSazonov iSazonov approved these changes

Assignees

@iSazonov iSazonov

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@daxian-dbw @iSazonov