Skip to content

Speedup InitialSessionState..cctor() #14283

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.

Sign up for GitHub

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

Merged
daxian-dbw merged 2 commits into from
Dec 4, 2020
Merged

Speedup InitialSessionState..cctor() #14283

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da0aefe
Speedup InitialSessionState..cctor()
iSazonov Nov 28, 2020
766efc6
Use Environment.TickCount64
iSazonov Dec 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/System.Management.Automation/security/wldpNativeMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ private static SystemEnforcementMode GetAppLockerPolicy(string path, SafeHandle

// AppLocker fails when you try to check a policy on a file
// with no content. So create a scratch file and test on that.
string dtAppLockerTestFileContents = AppLockerTestFileContents + DateTime.Now;
string dtAppLockerTestFileContents = AppLockerTestFileContents + Environment.TickCount64;
IO.File.WriteAllText(testPathScript, dtAppLockerTestFileContents);
Copy link
Contributor

@xtqqczze xtqqczze Nov 29, 2020

Choose a reason for hiding this comment

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

It seems the file name does not depend on the DateTime, but rather the file contents. So maybe we want to keep human date format

Copy link
Collaborator Author

@iSazonov iSazonov Nov 29, 2020

Choose a reason for hiding this comment

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

The file is created for automatic testing - no humans see and read the file.

Copy link
Contributor

@xtqqczze xtqqczze Nov 29, 2020

Choose a reason for hiding this comment

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

no humans see and read the file.

Perhaps it is for debugging purposes.

Actually, if no humans see the file do we require the datetime to be written in any form at all?

Copy link
Collaborator Author

@iSazonov iSazonov Nov 29, 2020

Choose a reason for hiding this comment

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

I don't know. I am only resolving the perf issue. I can only guess it is how AppLocker works. Perhaps @TravisEz13 or @PaulHigin could add more information.

Copy link
Member

@daxian-dbw daxian-dbw Dec 2, 2020

Choose a reason for hiding this comment

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

@TravisEz13 and @PaulHigin, can you please comment on whether we need DateTime to be added to the file content at all?

Copy link
Contributor

@PaulHigin PaulHigin Dec 2, 2020

Choose a reason for hiding this comment

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

The DateTime was included to add some randomness to the file contents. I think we still need it, but could change it to some other random value if computing it is quicker than DateTime.

Copy link
Member

@daxian-dbw daxian-dbw Dec 3, 2020

Choose a reason for hiding this comment

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

@iSazonov Maybe replacing DateTime.Now.Ticks with Environemnt.TickCount?

Copy link
Collaborator Author

@iSazonov iSazonov Dec 4, 2020

Choose a reason for hiding this comment

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

@daxian-dbw I have no objections. I see Environment.TickCount64 - maybe use it?

Copy link
Member

@daxian-dbw daxian-dbw Dec 4, 2020

Choose a reason for hiding this comment

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

That sounds fine to me, go ahead.

Copy link
Collaborator Author

@iSazonov iSazonov Dec 4, 2020

Choose a reason for hiding this comment

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

Done. The PR description has been updated too.

IO.File.WriteAllText(testPathModule, dtAppLockerTestFileContents);
}
Expand Down