Skip to content
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

Reuse engine process #17

Open
fitztrev opened this issue May 1, 2023 · 8 comments
Open

Reuse engine process #17

fitztrev opened this issue May 1, 2023 · 8 comments

Comments

@fitztrev
Copy link
Owner

fitztrev commented May 1, 2023

Don't start a new engine process on each request

@Disservin
Copy link

I think I've already messaged you about my engine process fix on discord, but i'll mention it here also ;)

The way I implemented it in my own is like this:

The communication is basically implemented in js (see https://github.com/Disservin/fast/blob/master/src/ts/ChessProcess.ts), by using the tauri shell and command api.
This js class object is simply persisted in the appropiate place.
There is a small problem though, tauri does not allow the execution of unknown shell commands, all commands have to be predefined in the config. This obviously wont work since we dont know the engine name at build time, thus I had to modify tauri itself and disable this security feature see: Disservin/tauri@00d961e it's just a small workaround.
There exists an issue in tauri already tauri-apps/tauri#5910 and an open but currently stale pr tauri-apps/tauri#7165.

I hope this helps ;)

@joshka
Copy link
Contributor

joshka commented Oct 24, 2023

Hey, I was thinking about implementing this if you're interested, by keeping the engine running in the background properly in rust. I wasn't sure what was happening with the engine settings PR which shakes a couple of things in that area up. Would you mind a PR for this, or prefer to get the other PR merged first?

@fitztrev
Copy link
Owner Author

@joshka That would be great! Ah yeah, that branch can be disregarded. This is a higher priority and I'll happily deal with any conflicts if this lands first.

@Disservin
Copy link

Tbf disabling that check and implementing it js side is probably way simpler and easier to do than to reimplement everything in rust :D

@joshka
Copy link
Contributor

joshka commented Oct 24, 2023

I did see your rationale on this. I have a general view that hacking around security controls is something that leads to suboptimal results when things change. So I think it's still worth doing in Rust.

@Disservin
Copy link

The security controls in this case anyway don’t offer much value… but yeah it’s a nasty workaround I admit and once the mentioned pr gets merged everything will be simpler anyway

@franciscoBSalgueiro
Copy link

I've already implemented this in my GUI:

The relevant code is here
https://github.com/franciscoBSalgueiro/en-croissant/blob/master/src-tauri/src/chess.rs

The way it works is basically this:

  • Rust exposes a command to start an engine and a a command to stop the engine.
  • The first command to start an engine stores the stdin handle (and a few other options) in tauri state and starts reading the stdout.
  • Every time there's a relevant uci message from stdout, an event is sent to the frontend.
  • Further commands that alter the uci options will get the stdin handle from state, write to it and then return. They'll not interact with the stdout as there's already one thread (the initial command) reading from it.
  • When the engine is stopped, the stdout handle is closed, so the initial thread will be responsible from removing it from state.

@hashedone
Copy link

I see this is basically nothing happening about this for over an year, and I think it is kinda important - it was the first thing I noticed when I peek at the code. Let me work on that, it should not be to difficult to fix and it would vastly improve performance (no need to compute the whole line every time!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants