ptazithos / glazewm-extra

This repository provides additional features for GlazeWM.
MIT License
31 stars 1 forks source link

feat: GlazeWM v3 upgrade attempt #9

Closed ehellman closed 2 months ago

ehellman commented 3 months ago

Hey!

Noticed some people talking on the GlazeWM discord about this project and that it was incompatible with version 3.

I want to preface this with the fact that I am NOT a Rust dev. So please, for the love of god, look at this code very critically :p But hopefully it might give you some ideas at least.

I split this PR into 2 commits. The first one is quite obvious, it just makes the project compatible with the V3 API again.

If you just stop there, there's quite a big issue. the command() function re-created the connection every time. I added some performance measurements and it took 2 seconds to start up that connection, meaning that the transparency happened on a 2 second delay for me every time. So I thought I would play around with an alternative.

I created a CommandExecutor struct that manages a single, reusable WebSocket connection (Stream) instead of creating a new connection for each command. Resource Efficiency was also something I was worried about with the earlier method. Constantly opening and closing connections is resource-intensive and could possibly lead to potential issues like port exhaustion. And of course, by using the same connection each time we reduce latency by 2 seconds at least, making the communication instant and the updates happen right away.

I also made it so that the stream isn't created right away, but when it's used the first time. This should probably be modified since you actually want to run it right away I think :D

This is a bit of deep water for me, but I wanted to make it have thread-safe sharing by using Arc<Mutex<Option<Stream>>> to allow the Stream to be shared safely across multiple threads and to handle the case where the Stream hasn't been initialized yet.

There is however, still a bug with this, as you can see: glazeextratitlebarbug

Haven't really figured out why this happens. Most things that you do in windows.rs is a mystery to me still :D

Hopefully you will at least find some inspiration here and I hope that it leads to something cool.

You can post here or find me as @eVonhell on the GlazeWM discord.

ptazithos commented 3 months ago

Thanks for the contribution! I need a while to review it.

ehellman commented 2 months ago

Thanks for the contribution! I need a while to review it.

No worries, don't feel any pressure to merge this either - just wanted to show what was required to change for it to work with GlazeWM v3. Noticed the delay issue on updating and thought I'd try to fix that too. :) We can keep building on this or you can pick what you like and implement it your own way, whichever is fine by me!

ptazithos commented 2 months ago

Good news! After appending a lot of logs to GlazeWM and Extra all night, I found the root cause for the extremely slow websocket connection establishment. It turns out that a connection to localhost will be first sent to ::1 before 127.0.0.1. But Glazewm only serves on 127.0.0.1. So, our requests take a while to find the correct dest. You could take this issue as a reference: https://github.com/dotnet/runtime/issues/31085 And these are related codes from both sides: Glazewm:

    let server_addr = format!("127.0.0.1:{}", DEFAULT_IPC_PORT);
    let server = TcpListener::bind(server_addr.clone()).await?;

Glazewm-extra:

    let stream = TcpStream::connect("localhost:6123").await?;
ptazithos commented 2 months ago

As for the long-existing connection, I want to keep the code as simple as possible. Let me revert the second commit first. Sorry for that.