Skip to content

Conversation

@link2xt
Copy link
Collaborator

@link2xt link2xt commented Feb 4, 2026

Alternative to #7795

@link2xt
Copy link
Collaborator Author

link2xt commented Feb 4, 2026

I have tested it on desktop, it works, events are frequently batched into 3 or 5 events for a single call. But no idea on actual impact on the performance, did not measure anything.

@WofWca
Copy link
Collaborator

WofWca commented Feb 10, 2026

I could not perform the tests under the same conditions so far, because we have some hacky event logging there, which doesn't work with this new JSON-RPC method.

https://github.com/deltachat/deltachat-desktop/blob/416f8f9e9e734f49de348c8a35db3c072155a741/packages/target-electron/src/deltachat/controller.ts#L114-L184

However, the delay is down to ~150ms without any logging (compared to ~1300ms in my MR).

I will try to fix the logging and then re-do the measurements.

@WofWca
Copy link
Collaborator

WofWca commented Feb 10, 2026

OK, I have fixed backend logging on DC Desktop and performed another 150 measurements on this patch. The average is 238 ms, with standard deviation of 89.97 ms.
Admittedly, there were ~3 <50ms outliers in my measurements, and I discarded them.

For good measure, I also re-measured the delay with the old non-batch API, but without backend logging, and got 558.81 ms on average and 196.16 ms of standard deviation over 37 samples.
With backend logging and the old non-batch API we are back to 1172.61 ms on average with std deviation of 155.76 ms over 24 samples.

I also tested this MR without backend logging, and the delay is at blazing 130 ms on average with std deviation of 6.10 ms!

So, no matter if logging is on or off, this is an improvement!

Copy link
Collaborator

@WofWca WofWca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is technically not a breaking change, but, since DC Desktop is using some hacks, it would need to get adjusted. Anyways, I already have the code ready for that.

I didn't look to closely at the Rust part of the code, but it looks sane, and seems to work. The JS code looks trivial.

@WofWca
Copy link
Collaborator

WofWca commented Feb 10, 2026

So this is technically not a breaking change, but, since DC Desktop is using some hacks, it would need to get adjusted.

Regarding this: I think I could make an MR that adds support for this batch API, before this Core MR is merged, so as to not get other devs who always use latest core with latest desktop angry.

@link2xt link2xt force-pushed the link2xt/qqvpnqqrkmyv branch from ec7b44a to 129ea2d Compare February 11, 2026 08:17
@link2xt
Copy link
Collaborator Author

link2xt commented Feb 11, 2026

I added doc comment and limited the number of returned events to 100 per batch. It is unlikely ever at 100 events, just to have some limit on memory usage.

WofWca added a commit to deltachat/deltachat-desktop that referenced this pull request Feb 11, 2026
See chatmail/core#7825.

This MR ensures that we properly handle events
even when they are retrieved with the new `getNextEventBatch()`
Core API, which is going to begin being the case
when that MR is merged.
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

Successfully merging this pull request may close these issues.

2 participants