-
Notifications
You must be signed in to change notification settings - Fork 60
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
Enhancement: Matrix commander is slow, it seems to temporarily hang due to syncing #91
Comments
Getting similar issue, even 2 minutes wait sometimes. I was told it's maybe because I'm using matrix.org homeserver which is known to be slow, but I don't see that slowness when sending messages using element. |
yes and this is in a matrix.org server |
I tried to debug a little bit and dig deeper in the code but I didn't found any evident issue, I think I need to profile the python code |
Also might be worth bench-marking using other homeservers or even a local one. |
@ users See comments below line 5805. Syncing means updating the on-disk cache, by downloading all events from all rooms since the last app execution. This typical workflow of any Matrix client goes unnoticed in a graphical client, because of user interaction (browsing to the room, typing the message). If you want encryption, syncing is necessary (encryption requires downloading state events, you need to know which devices are trusted or not, and matrix-nio is designed to require a sync if you want encryption).
@ myself There is an unsupported way in matrix-nio to send encrypted without syncing. On my test setup, I did the initial sync. For the following executions, instead of the sync, |
Note from my testings: Performing a bulk ban in different rooms is just taking a few seconds per room, nothing compared with sending a message. |
OK, that can be done.
Doable. Yes. But this allows the user to shoot oneself into the foot. What is your opinion? Please share! |
matrix-nioI search for
The only case where the examples sync before sending a room event is when encryption is needed, in ProtocolThe specification to send an event is
The parameters are: homeserver, room id, transaction id, access token, message body. Uploading a file does not touch a room (you upload to a server, not a room) and is at 1.1.9.2 under Server and databaseA client's ability to make the server (Synapse, DB, ...) not follow the protocol (corrupt, alter the room's past, alter future behavior, ...) is a security issue on the server and not a fault of the client. |
That the documentation sends before syncing also implies that you can send without syncing at all, because a network error during the later syncing would otherwise break / corrupt matrix-nio. |
Thank you @opk12 for your research and your summary. The matrix-nio examples do not convince me (as I wrote some of these examples myself) and they were written to keep things extremely simple, not to be best-practice code. The Protocol example convinces me a lot more. I take it, you @opk12 are voting in favor of a Should there be a Or should there be no user option, no command-line option, but Please share your Thoughts? Input? Opinions? |
If that's the case, this should be written next to each example, because people are jumping to the examples through links (not read the whole page) and use them as reference (copy-paste). But then, is it possible that matrix-nio needs syncing to send? For example, in theory it could
|
Since I do not dare to give a definite answer, in short I am not sure, the best way forward might be to raise the question as an issue on
plus additional narrow questions:
|
1Other possible questions that come to mind:
Because it might be possible, but with various levels of degraded functionality or security (which may be relevant or not, depending on the use case). I don't know who maintains matrix-nio, but pointing out that it was you who wrote some of those examples might help if anyone who answers got a misconception based on reading the documentation. However, I hope that the fact that the examples were kept in the docs until today implies a partial acknowledgement. 2I did not test encryption with matrix-commander, but do I understand it correctly that this message should be sent encrypted?
If yes, then sending cannot default to auto-detect, and requiring an option for all operations would be consistent and more readable (when reading a command, I do not need GitHub just to remember whether it auto-skips) and would give the feeling that the program defaults to safety. What about |
I just looked at my own source code, and back in the day I put in some comments as I wrote the code for Around line 5881 you will find comments like:
There are 2 API calls: So, just blindly sending without a
This is not a complete list of what can go wrong. But At the same time, the code is written such that even if the sync fails, the send will take place. Meaning, right now there is no 100% guarantee the sync will be successful. What I am implying is what @opk12 said before: sending without syncing might lead to some error (room id not found, key not found) but it will not lead to corruption on the server. This is another way of saying: it might not be so dangerous to skip the sync, especially in cases where no new rooms where created, no new keys were created, etc.
What is your preferred option name? Then to make it even more complicated: the
|
As you can see I have opened this issue on |
But note that all this is still about encryption. (And I wonder whether "get room ids" was related to some error message arising from
Once the local cache is updated, all operations will be affected. If "sync after X but before Y" is wanted, then it's clearer to call m-c once for X and once for Y.
I think it mirrors the |
I did some testing of my own today. The setup matters a lot, so your mileage may vary drastically. I tested only short send actions, like What I found is:
So, even turning sync off, for sending single messages, this only reduces the total time by at best 33%. Again, mileage may vary drastically. But it is not a magic bullet and no, now it does not go in the blink-of-an-eye. For the time being the default is |
See https://github.com/8go/matrix-commander-rs An idea to improve performance 😄 Talk about it to your friends, post about it, spread the word and provide PRs. 🙏 And give it a ⭐ if you like the idea. |
Eh but I fear that the network is the problem. The language impacts CPU usage, but m-c is spending a negligible time on the CPU; the usual symptoms of being CPU-bound are heating the computer, slowing down the system, or that the stack trace that appears at Ctrl + C contains some function that does a very long and tight loop (but that's not the case for m-c or for matrix-nio). That said, it's not impossible that matrix-nio (as opposed to matrix-commander) does additional (necessary, or unnecessary) HTTP calls, so while I would not bet that switching the library could improve performance, I'd be happy to be proven wrong. |
Thank you! I updated my script to use it. |
Another tip to avoid performance penalty: do NOT use room aliases, but use room ids instead. Room aliases need to be looked up, adding another REST call to the server. |
If you just want to login, verify and send a text or a file, you can now use the Rust version. Rust version cannot listen (hoping for PRs). Rust version is also not optimized ( Feedback is welcome. Check out: |
Hi,
I'm testing this software and no matter what I try to do it always freeze for one minute after finally doing what I want.
I wanted to send a message to a specific room, I issued this command :
However this is taking 67 secs! for a single message? Where it did go all wrong?
It's seems to be waiting for some thing between
2022-08-22 08:14:01,495: DEBUG: matrix-commander: Rooms are: ['!REDACTED:matrix.org']
and
2022-08-22 08:15:37,089: DEBUG: matrix-commander: stdin is not ready. A pipe could be used, but pipe could be empty, stdin could also be a keyboard.
My environnement:
Anyway thanks for your work
The text was updated successfully, but these errors were encountered: