Issue 55685 processname optimisation

[SteveDunn]

Fixes #55685.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	SteveDunn
Assignees:	-
Labels:	area-System.Diagnostics.Process, community-contribution
Milestone:	-

[stephentoub]

Thanks for working on this.

Can you share performance numbers? In particular, how does this impact the cost of a) iterating through every process and getting its name, and b) iterating through every process and getting both its name and its WorkingSet64? I ask because this is not only changing what Win32 function is used to get the process' name, it's also changing whether retrieving the name also retrieves other state.

[SteveDunn]

Thanks for working on this.

Can you share performance numbers? In particular, how does this impact the cost of a) iterating through every process and getting its name, and b) iterating through every process and getting both its name and its WorkingSet64? I ask because this is not only changing what Win32 function is used to get the process' name, it's also changing whether retrieving the name also retrieves other state.

Very welcome! Good idea re. benchmarking. I'm happy to do this, although I think I would need some assistance. I'm somewhat familar with benchmarking, but I've been having difficulties consuming these changes from a standalone app outside the repo.
I'd be happy to be given instructions/guidance on how to do this, alternatively, if someone wants to jump in and do it, then I'm happy with that too (especially since I am away for a week now)

[SteveDunn]

@am11 / @stephentoub - are those failing checks a blocker? They don't look related to the changes in this PR.

Also, are the benchmarks required before this PR can be approved? I won't have time to do them, but can do them in a week or so.

[stephentoub]

Also, are the benchmarks required before this PR can be approved?

Yes. The purpose of this PR is improving performance. That needs to be validated for this to be worthwhile churn, and it needs to validate that it doesn't regress other usage.

Thanks.

[SteveDunn]

Thanks for working on this.

Can you share performance numbers? In particular, how does this impact the cost of a) iterating through every process and getting its name, and b) iterating through every process and getting both its name and its WorkingSet64? I ask because this is not only changing what Win32 function is used to get the process' name, it's also changing whether retrieving the name also retrieves other state.

@stephentoub - here are the benchmarks as requested:

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22000
Intel Core i7-7700K CPU 4.20GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK=7.0.100-alpha.1.21467.19
  [Host]     : .NET 5.0.10 (5.0.1021.41214), X64 RyuJIT
  Job-YJXSLY : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
  Job-NVCLTE : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT

IterationCount=15  LaunchCount=2  WarmupCount=10  

Method	Job	Toolchain	Mean	Error	StdDev	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
CurrentProcess_ProcessName_Only_Just_Once	Job-YJXSLY	main	2,615.28 μs	52.515 μs	78.602 μs	1.00	0.00	-	-	-	3,186 B
CurrentProcess_ProcessName_Only_Just_Once	Job-NVCLTE	PR	43.52 μs	0.642 μs	0.960 μs	0.02	0.00	0.1221	-	-	672 B
CurrentProcess_ProcessName_AfterWorkingSet64_Just_Once	Job-YJXSLY	main	2,584.42 μs	37.714 μs	55.280 μs	1.00	0.00	-	-	-	3,282 B
CurrentProcess_ProcessName_AfterWorkingSet64_Just_Once	Job-NVCLTE	PR	2,411.35 μs	29.618 μs	43.414 μs	0.93	0.03	-	-	-	3,290 B
CurrentProcess_ProcessName_Only_Multiple	Job-YJXSLY	main	3,168.35 μs	107.675 μs	154.424 μs	1.00	0.00	1679.6875	-	-	7,035,146 B
CurrentProcess_ProcessName_Only_Multiple	Job-NVCLTE	PR	432.88 μs	11.265 μs	16.860 μs	0.14	0.01	1679.6875	-	-	7,032,632 B
CurrentProcess_ProcessName_AfterWorkingSet64	Job-YJXSLY	main	3,019.24 μs	51.647 μs	77.303 μs	1.00	0.00	1683.5938	-	-	7,051,226 B
CurrentProcess_ProcessName_AfterWorkingSet64	Job-NVCLTE	PR	2,880.19 μs	38.718 μs	55.527 μs	0.96	0.03	1683.5938	-	-	7,051,234 B
CurrentProcess_ProcessName_BeforeWorkingSet64_Multiple	Job-YJXSLY	main	2,962.78 μs	28.560 μs	42.747 μs	1.00	0.00	1683.5938	-	-	7,049,226 B
CurrentProcess_ProcessName_BeforeWorkingSet64_Multiple	Job-NVCLTE	PR	3,011.14 μs	40.898 μs	58.655 μs	1.02	0.03	1683.5938	-	-	7,049,618 B
AllProcesses_ProcessName_Only_Multiple	Job-YJXSLY	main	11,225.78 μs	196.514 μs	288.048 μs	1.00	0.00	33343.7500	234.3750	109.3750	142,780,460 B
AllProcesses_ProcessName_Only_Multiple	Job-NVCLTE	PR	10,814.94 μs	154.934 μs	227.100 μs	0.96	0.03	31453.1250	328.1250	156.2500	134,779,175 B
AllProcesses_ProcessName_AfterWorkingSet64_Multiple	Job-YJXSLY	main	12,916.11 μs	137.887 μs	197.753 μs	1.00	0.00	39828.1250	234.3750	109.3750	169,803,168 B
AllProcesses_ProcessName_AfterWorkingSet64_Multiple	Job-NVCLTE	PR	12,188.59 μs	230.248 μs	344.624 μs	0.95	0.03	37140.6250	312.5000	156.2500	158,539,815 B
AllProcesses_ProcessName_BeforeWorkingSet64_Multiple	Job-YJXSLY	main	12,849.65 μs	179.079 μs	268.037 μs	1.00	0.00	39828.1250	312.5000	156.2500	169,818,559 B
AllProcesses_ProcessName_BeforeWorkingSet64_Multiple	Job-NVCLTE	PR	11,890.46 μs	99.702 μs	149.229 μs	0.93	0.02	37156.2500	312.5000	156.2500	158,552,346 B

[SteveDunn]

Do I need to do anything else for this PR? I can see some of the checks have failed, but I don't think it's related to this code (but could be wrong)

[adamsitnik]

Overall looks good, but we need to improve handling of the edge cases (please see my comments). Thank you for your contribution @SteveDunn !

[adamsitnik]

nit: extra empty line

Suggested change

	}
	}

[adamsitnik]

thank you for keeping the exception consistent with existing implementation! 👍

[adamsitnik]

LGTM, once again thank you @SteveDunn !