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

Desktop: Speed up WebDAV/Nextcloud sync by ~30% (or even more) #4625

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

roman-r-m
Copy link
Collaborator

(Sorry for the catchy title, couldn't help myself.)

Due to a typo Keep-Alive wasn't sent so each time http connection has to be re-established from scratch).
I have tested this with a server in my local network using plain http and got these results:

  • Before the change
21:10:38: CommandService::execute: synchronize false
...
21:13:34: Sync: finished: Synchronisation finished [1614978638697]

almost exactly 3 minutes

  • After the change
21:21:15: CommandService::execute: synchronize false
...
21:23:19: Sync: finished: Synchronisation finished [1614979275473]

~2 minutes, so 30% speedup. For https I expect the effect would be even greater. Not too bad for a one-symbol change.

Total synced volume was 188,621,312 bytes, items:

main-html.js:48 21:23:19: Operations completed: 
main-html.js:48 21:23:19: createRemote: 1264
main-html.js:48 21:23:19: fetchingTotal: 1265
main-html.js:48 21:23:19: fetchingProcessed: 1265
main-html.js:48 21:23:19: Total folders: 27
main-html.js:48 21:23:19: Total notes: 850
main-html.js:48 21:23:19: Total resources: 187

PS
Why is Linux-only? And why is httpAgent only used for WebDAV?

@tessus tessus added the desktop All desktop platforms label Mar 7, 2021
@roman-r-m
Copy link
Collaborator Author

Ran another test with Nextcloud over https:

  • Before the change: ~1h 15 min to sync the same set of notes as above
  • After the change: 35 min

@laurent22
Copy link
Owner

See here about why it's only enabled on Linux: #1931

@roman-r-m
Copy link
Collaborator Author

It actually does not explain why it's Linux only, the issue that the author of that PR was fixing (#1023) references Linux, presumably because the user who raised it was using Linux.

I will test it on my Windows machine (too bad I've deleted my dev environment there), but I do not see a reason why it should not work there.

@laurent22
Copy link
Owner

There was also a concern about whether HTTP agents work with React Native or not.

@roman-r-m
Copy link
Collaborator Author

roman-r-m commented Mar 10, 2021

There was also a concern about whether HTTP agents work with React Native or not.

That I have not tested. This PR just fixes a typo that made this original change pointless, even on Linux.

I still think it should be useful on Windows too, just having trouble building the desktop app at the moment.

@laurent22
Copy link
Owner

Ok I guess we can merge anyway now, just for the typo fix. It's true it might work just fine on other platforms too, but we'd need to test.

@laurent22 laurent22 merged commit 7eb9305 into laurent22:dev Mar 11, 2021
@roman-r-m
Copy link
Collaborator Author

It looks like Windows is also affected by the same issue. As you can see from the screenshot Joplin closes the connection after each request, all those SYN/ACK/FIN are just time wasted . I'll see if the same fix applies on Windows.

image

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

Successfully merging this pull request may close these issues.

3 participants