LionyxML / auto-dark-emacs

Auto-Dark-Emacs is an auto changer between 2 themes, dark/light, following MacOS, Linux or Windows Dark Mode settings
GNU General Public License v2.0
140 stars 34 forks source link

Slightly freezes on Windows #30

Closed ticolensic closed 1 year ago

ticolensic commented 1 year ago

I'm new to emacs (well, 10 years of trying to adopt and adapt). I tried this plugin on Windows, and it slightly freezes every, I presume, run of the powershell. I sorta know why, because of a synchronous call (and this is the reason I was postponing switching to Emacs). I'm not sure if this is fixable, but it is still an issue. I can take a look myself if I have time.

I'm using Windows 10 22H2 and Emacs 29.

LionyxML commented 1 year ago

Hello there!

Thanks for trying it out!

I have no access to a Windows machine right know. But what do you think it would be?

It seems to be related to powershell being called synchronously and your machine running it with a little delay.

Would you like to take a look at it @seagle0128? Does this happen to you too?

seagle0128 commented 1 year ago

I am using Emacs 29 on Windows 11, and I didn't observe this issue on my laptop.

ticolensic commented 1 year ago

I'll try and record a gif. I have something in mind how to fix it.

McPhale commented 1 year ago

I'm having the same problem; took a look at the emacs process under process explorer and it does seem to be the powershell calls causing it to hang.

ticolensic commented 1 year ago

I finally was able to record it, mp4 instead of gif (so that there's enough frames to catch it). dropbox, 883kb You can see this effect on "b" lines 1, 3 and 4.

It is definitely the powershell script, you can run it manually: it takes a second to finish. I was thinking of threading it and putting the result to a global variable. I know, I know, but this could fit here, sorta a thread-unsafe queue for the poor.

seagle0128 commented 1 year ago

@ticolensic I guess it's due to the gc. Try increase gc-cons-threshold.

LionyxML commented 1 year ago

@ticolensic have you tried increasing the gc-const-threshold? Any luck?

McPhale commented 1 year ago

I had mine bumped up to 100000000 before installing the package and still had the same issue, so I doubt that's it? Happy to do more troubleshooting.

quienn commented 1 year ago

PowerShell is really that slow. So, my suggestion is ditch it and use cmd instead.

Something like this should do the trick:

reg query HKCU\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize /v AppsUseLightTheme

It's faster 'cause you're getting the value directly from the registry, not using a wrapper around it.

It should return the following output:

HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize
    AppsUseLightTheme    REG_DWORD    0x0
seagle0128 commented 1 year ago

PowerShell is really that slow. So, my suggestion is ditch it and use cmd instead.

Agree. powershell is slower than cmd generally. The func auto-dark--is-dark-mode-powershell should be optimized.

quienn commented 1 year ago

After a couple hours trying to get my head around Elisp I got this little snippet to work:

(defun auto-dark--is-dark-mode-powershell ()
    (string-equal "0x0"
                  (car (reverse (split-string (string-trim (shell-command-to-string "reg query HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize /v AppsUseLightTheme | findstr 0x")))))))

I know, it sucks. :smile: It can be optimized, but I don't have the time to do it myself.

For the people like me that were having problems using the PowerShell implementation, putting this just after auto-dark-mode should give a huge performance boost.

Edit: Apparently, string-split does not exist in Vanilla Emacs. 😃 I was using Doom Emacs when testing it.

seagle0128 commented 1 year ago

Edit: Apparently, string-split does not exist in Vanilla Emacs. 😃 I was using Doom Emacs when testing it.

Using compat should work.

LionyxML commented 1 year ago

I'd be more than happy if anyone have the time to make a PR :)

LionyxML commented 1 year ago

Thanks @quienn , it is now merged!