-
Notifications
You must be signed in to change notification settings - Fork 81
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.
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
Multithreading with Runspaces #150
base: master
Are you sure you want to change the base?
Conversation
- Made info_disk function activate in another thread in an attempt to decrease lag caused by it
- Should be "*easily*" expandable to include entire script for large performance gain
Please don't make changes to the function definition (revert changes on line 801 and 829). How will you implement this for the rest of the functions? |
- Made the runspaces implementation expandable - Implemented runspace pools which speeds up multiple runspaces significantly - Single runspace slightly faster too - Added array in which a runspace is created for each entry that is enabled in the config - Allows for some entries which may be faster without runspaces to be executed on the main thread
The change made on line 801 is a temporary fix to an issue that was occurring and will be reverted when I fix it The change on line 829 was made to prevent me from having to pass get_level_info along with the variables and function it depends on into the runspace, which I think would be wasteful due to its use of the function being minimal and it not using the function or variables it depends on. Instead, I took the single part of the function it used and directly implemented it where the function declaration was. This also likely saves a little bit of performance by bypassing all of the unnecessary checks the function performs.
As seen in the commit I just made, I implemented runspace pools and created a loop that will create and execute the runspaces which will benefit from being multithreaded (some functions likely will be faster outside of runspaces due to their speed and the overhead of runspaces). I will convert the rest of the functions for runspaces likely just like how I did for disks:
|
No, you have ignored the variable
To prevent rewriting major parts of the code, you can directly use a function's definition (by assigning |
Oh, I seem to have missed that. I'll see if I can add that in or just pass the function and variables in.
Yeah, that might work. I'll see what I can whip up when I have the time Also, progress might be slow due to me being busy due to college. But I intend to get this done as I use this program all the time |
Sure, no worries. This is a pretty significant change, so please take all the time you need :) |
- Created $Vars and $Funcs - Contains all functions and variables the runspaces might need
- Instead of having a scriptblock and helper info function, the original info functions are converted to a scriptblock and the helper function was moved into the $info loop - Functionally the same while preserving the original functionality of the script - Added config option and parameter $runspaces where if enabled(default), runspaces are used. And if disabled, runspaces aren't used. - Disabling runspaces is very useful for debugging as step through in PowerShell ISE doesn't show operations within a runspace.
Implemented in the last commit. |
- Adds $Vars and $Funcs into the runspace - Adds helper script into the beginning of the runspaces to declare needed variables and functions - Currently declares all of the variables and functions for every runspace, which is inefficient. Could be solved with much increased complexity if performance issues arise from it. - Reverted changes to info_disk as the issues were solved by passing variables in
Took a bit of work, but I figured it out eventually.
Reverted in the most recent commit Theoretically, all I need to do now is start adding functions to $RunspaceOps and they'll hopefully just work. Probably will run into an issue somewhere but I guess one can hope |
- Fixed bug on line 1160 - Closes and disposes of runspaces and the pool as it is good practice to prevent issues and conflicts
It seems that the only functions with issues are memory and battery. If I can't figure out what's wrong, I'll just have them disabled by default in $RunspaceOps. I'll post my final speeds soon |
Speeds: Everything Enabled: Everything Enabled Except "ps_pkgs", "cpu_usage", and "locale": Nothing Enabled: Defaults: My Config: |
In terms of speed, running with runspaces is beneficial in almost every configuration except those where one has every 'slow' operation disabled. When only near-instant operations are enabled, the slight overhead of runspaces overpowers the speed increase of parallelization, leading to a slight performance decrease. So runspaces cause a larger performance increase the longer winfetch takes to run but also causes a small performance decrease if it runs fast enough. I think most people will see either a performance increase or an almost unnoticeable performance decrease, which I think is enough to keep the default as enabled. |
- Set $RunspaceOps to every operation that doesn't error - Having memory and battery enabled cause really weird issues sometimes
I think I have figured out what is causing the issues with the info_memory and info_battery functions. It seems that for some reason when a call to get_level_info exists within those two functions, powershell seemingly "forgets" some combination of these things:
The quantity of errors and when they occur seems to be random, except that they all occur from line 1195 down. Any execution of the function causes the issue, even by calling the function using |
- Added some comments and tweaked some existing ones - Removed $t from $Vars as it was unused
I think it's best to keep memory and battery out of runspace execution for now as it seems that fixing their issues would require a lot more work than it's worth. |
@rashil2000 Unless you have any recommendations, I think the pull request is ready to be merged. |
Okay, will review it in some time |
TODO:
Fix it