X Tutup
Skip to content

Add CommandLine property to Process#12288

Merged
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
iSazonov:ps-commandline
May 29, 2020
Merged

Add CommandLine property to Process#12288
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
iSazonov:ps-commandline

Conversation

@iSazonov
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov commented Apr 9, 2020

PR Summary

Fix #3322 on Windows and Linux. For MacOs we will open new tracking issue to implement later.

PR Context

PR Checklist

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Apr 9, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.2 milestone Apr 9, 2020
@ghost ghost assigned adityapatwardhan Apr 9, 2020
@iSazonov iSazonov marked this pull request as ready for review April 9, 2020 18:21
@adityapatwardhan
Copy link
Copy Markdown
Member

@iSazonov can you give a before and after for a perf comparison? Call Get-CimInstance or Get-Content for every System.Diagnostics.Process object sounds expensive.

The test can be as simple as:

Measure-command { 1..100 | % { get-process } }

@iSazonov
Copy link
Copy Markdown
Collaborator Author

@adityapatwardhan It is "on demand" property - you evaluate it only if you explicitly call the property. But if you call you get a slowdown. It is not in default output so users don't see slowdown in common scenarios.
Notice, I found WMI is very fast on latest Windows versions (Windows 10 and Windows Server 2019). If someone provides feedback that it's still slow we could consider a P/Invoke.
On Linux /proc works fast.
For MacOs I do not know API. I am going to open new tracking issue for this after the PR will be merged.

@SteveL-MSFT
Copy link
Copy Markdown
Member

For both Linux and macOS, I wonder if it would be ok to run ps to cache the command lines and map it to the object?

@iSazonov
Copy link
Copy Markdown
Collaborator Author

Using /proc is simple and reliable on Linux so no need to create a workaround.
I haven't Mac and can not create even a workaround. Nonetheless I hope there is a good API for that.

@adityapatwardhan
Copy link
Copy Markdown
Member

@TravisEz13 @JamesWTruher Do you know of a macOS workaround for this?

@adityapatwardhan
Copy link
Copy Markdown
Member

@iSazonov I am not sure if we should take this PR without having a solution for all platforms. This introduces gotchas and makes cross platform scripting difficult.

@iSazonov @SteveL-MSFT - thoughts?

@iSazonov
Copy link
Copy Markdown
Collaborator Author

My plan is to open a tracking issue for MacOs. We have a lot of time before release and can investigate how enhance the feature on MacOs.

@powercode
Copy link
Copy Markdown
Collaborator

powercode commented May 8, 2020

The APIs will probably require some native coding on the mac.

We can probably use the same apis as top.

https://opensource.apple.com/source/top/top-111.20.1/libtop.c.auto.html

@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost
Copy link
Copy Markdown

ghost commented May 27, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

@adityapatwardhan
Copy link
Copy Markdown
Member

@iSazonov Please open a tracking bug for macOS and also open a issue for documentation as per the PR template.

@ghost ghost removed the Review - Needed The PR is being reviewed label May 28, 2020
@iSazonov
Copy link
Copy Markdown
Collaborator Author

@adityapatwardhan Tracking issue #12832

As for doc issue - do we really need to document this? It seems we haven't docs for extended properties at all.
/cc @sdwheeler

@sdwheeler
Copy link
Copy Markdown
Collaborator

@iSazonov No, we don't document the extended properties. But this is worth adding to the release notes.

@adityapatwardhan adityapatwardhan merged commit c7455fd into PowerShell:master May 29, 2020
@adityapatwardhan
Copy link
Copy Markdown
Member

@iSazonov Thank you for your contribution!

@iSazonov iSazonov deleted the ps-commandline branch May 31, 2020 04:56
@ghost
Copy link
Copy Markdown

ghost commented Jun 25, 2020

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend Process with CommandLine property

6 participants

X Tutup