GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — Reduce string allocations when formatting file system objects. by powercode · Pull Request #8831 · PowerShell/PowerShell Conversation
|
|
Staffan Gustafsson (@powercode) Please have a look at the test failure. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| internal void SetModule(PSModuleInfo module) | ||
| { | ||
| Module = module; | ||
| _fullName = null; // clear the cached FullName |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please remove the comment. Better use _cacheFullModuleName name.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
But it isn't a full module name. That name would be misleading. But sure, I can remove the comment.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If it is not a full name what about _cachedModuleName?
I see there comment "Gets the name of the provider." So the name could be appropriate - _cachedProviderName.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The property is FullName, so it makes sense to me that the backing property is _fullName.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I tried to insert your comment to the name that it is a cached :-)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Awesome. Out of curiosity: Do you have time measurements of cases where this improvement is noticeable? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Christoph Bergmeister (@bergmeister) It's mostly in allocations - it generates lots of extra strings, like 2 strings per item in the pipeline. I'm reasoning more in terms of steadily removing silly allocations, and fixing the worst offenders perf-wise, and eventually we will have a more snappy shell. The image below show the dotmemory view of strings allocated, grouped by callstack, when looking at the first 50000 items in the windows dir. It wasn't the biggest, but it was one of the bigger, and it was low hanging fruit. Does it make sense? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks Staffan Gustafsson (@powercode). Yes, makes sense. I agree that continous improvement will make it better over time. :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Christoph Bergmeister (@bergmeister) It may seem like I'm just doing random changes, but I actually measure things now and then :) dotTrace and dotMemory are almost always running on my machine. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Staffan Gustafsson (@powercode) I was not questioning it, I was just curious for my own education (because knowing this also means that one will know the scenarios where PSCore is stronger than Windows PowerShell meaning that I could recommend an upgrade to clients). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Christoph Bergmeister (@bergmeister) Windows PowerShell is still more efficient than PowerShell Core |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Christoph Bergmeister (@bergmeister) I'm currently working on optimizations for the formatting system, the filesystem provider, etc that makes it out-perform windows powershell with a huge margin, often like 4x. And with a memory footprint reduction of the similar size. Import-Csv is problematic, since our property abstraction is heavier that I would like it to be. Especially in cases like that, where we actually have a table, we wouldn't need to store all the metadata for each object. I haven't given it so much thought - just passed by it last fall when doing related work. I have an idea that I like to try out for Import-CSV - could drastically reduce the memory footprint. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I was thinking about compiling to classes too and found some problems in the approach. One problem could be solved by #8852 (not trivial). After finding IDataView #8855 I suppose this is preferred way we should start research. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…Shell#8831) * Reduce string allocations when formatting file system objects by caching FullName. * Clear cache FullName when the module changes.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.