Steve Lee (@SteveL-MSFT) To be honest, I've done an embarrassingly small amount of testing. Except for running Start-PSPester.

Note to reviewers: Bruce Payette (@BrucePay) Dongbo Wang (@daxian-dbw)

I tried another route in this second PR, by adding another delegate for member lookup. It is less intrusive in the previous functionality, so the risk should be lower.
I wonder if you, when you review this, can try to think about if the same things could be accomplished in a smarter way? I'm adding a way to check for the existence of any property in a reasonably efficient way. But in reality, we only use it to check if there exist any property other than our magic remoting note properties. for very specific properties. (The different stream names, and the remoting properties). In the stream case, it is also important which stream it is, but we don't need a PSNoteProperty for that. For example, the 7 boolean flags in PSObject could become bit fields. Maybe we could encode this information there?

I have refactored PSObject to use flags instead of Booleans. I also removed some unused fields, and have plans for using the available padding for speeding up the handling of the remoting properties. I added a field for the stream to use by the formatting system - this was previously magically named PSNoteProperties - and the lookup of these was expensive during formatting.

As you can see, I've implemented a special case of a the Linq operator FirstOrDefault, but I chose not to make this an iterator as I wanted to keep allocations to a minimum. Not sure it's the right call, just trying to explain where I come from :)

I'm a bit weak on the serialization, so that is an area that could need some scrutiny to see if I've messed anything up.

the 7 boolean flags in PSObject could become bit fields. Maybe we could encode this information there?

It can break remoting/serialization and backward compatibility.

It can break remoting/serialization and backward compatibility.

I think that is just an implementation detail. We may need to handle that in the deserializer - but good to keep in mind when doing it.
But I don't think these fields are serialized at all - they seem to be set in the constructor when deserializing.

I have a sample implementation here - https://github.com/powercode/PowerShell/tree/PSObjectFlags

Edit:
All but the remoting implementation is merged into this PR. I know a bit to little about the remoting to be confident to make that change now.

I have a sample implementation here - https://github.com/powercode/PowerShell/tree/PSObjectFlags

Looks great at first short review! (We'll need measurement of CPU/memory benefits. And main complicity is that we have to manually test remoting and serialization.)

Ilya (@iSazonov) Steve Lee (@SteveL-MSFT) Any chance we can turn of the hungarian notation warnings from Code Factor? It has nearly 100% false positives.

I don't intend to fix the remaining code factor issues.

Staffan Gustafsson (@powercode) I believe we're eventually moving off CodeFactor and over to Codacy. Agree the false positives on hungarian are annoying.

Codacy reports tons of false-positives. We need to merge Dongbo's PR that resolve most CodeFactor issues.

Rain Sallow (/u/ta11ow) (@vexx32) I don't look but if you comments is for copy-pasted (not new) code we shouldn't fix it in the PR - it is complicate code review.

Looks good!

It seems we need rename the PR from

Faster formatting by having better primitives on PSObject

to

Improve performance by having better primitives on PSObject

Can we move style changes to #9149 to follow our best practice?

Ilya (@iSazonov) I have reviewed all changes in this PR and all look good. Separating the refactoring changes will not only cost extra efforts to resolve conflicts but also require another pass of review to make sure no mistake made during those changes. I don't think it worth the effort.

All issues reported by Codacy are false alarms.

I don't think it worth the effort.

We agreed to write this in our guide. This allows us to maintain a clear history of functional changes without style ones. Of course it takes effort.
It is not blocking comment.

This allows us to maintain a clear history of functional changes without style ones. Of course it takes effort.

I agree that will get us a clear view of the functional change history. But I think it's more appropriate to bring this up at very early stage of the PR life cycle.
When we reach the end stage of a PR, it's too costly to do this.

I think it's more appropriate to bring this up at very early stage of the PR life cycle.

At early stage we haven't all style changes.
From my experience we ready to do this near to finish - and rebase (1) to get new tests, (2) remove unrelated changes.

The main benefit is that the commits are clean.
Otherwise in a few months the details will be forgotten and we will have to figure it out again, not to mention those who will do code analysis in this place for the first time.

When we reach the end stage of a PR, it's too costly to do this.

In previous powercode's PR you did this :-)

Staffan Gustafsson (@powercode) Thanks for the great improvement!

Staffan Gustafsson (@powercode) This is outstanding work here!