magic-wormhole.rs icon indicating copy to clipboard operation
magic-wormhole.rs copied to clipboard

Increase chunk size from 4KB to 16KB to fix performance issue.

Open HuakunShen opened this issue 9 months ago • 1 comments

Python and golang implementations have 16KB chunk size but the rust version has 4KB.

I experienced a slower speed when sending files from a MacBook with M1 pro chip.

Looks like the issue is chunk size. Once I update chunk size to 16KB the speed aligns with the python and golang implementations.

For more information, see issue https://github.com/magic-wormhole/magic-wormhole.rs/issues/224

I found the potential issue. I reviewed the code of the python and golang version with a debugger and found that the rust implementation has a different chunk size for each send. Python and Golang implementations both send 16KB at a time while the rust version sends 4KB at a time.

After setting chunk size to 16KB I get full speed.

Python

File sending starts from here https://github.com/magic-wormhole/magic-wormhole/blob/02407c4aa4cc3f8d8cd01d549fdc72a5f5d77010/> src/wormhole/cli/cmd_send.py#L442

fs = basic.FileSender()

with self._timing.add("tx file"):
    with progress:
        if filesize:
            # don't send zero-length files
            yield fs.beginFileTransfer(
                self._fd_to_send,
                record_pipe,
                transform=_count_and_hash)

The chunk size is defined in twisted package twisted.protocols.basic.FileSender

File chunks are read here https://github.com/twisted/twisted/blob/02a2b658cd1ade5d7f41f97d898913686313e615/src/twisted/> protocols/basic.py#L892

CHUNK_SIZE is a constant defined as CHUNK_SIZE = 2**14 (which is 16384bytes, 16kB) at https://github.com/twisted/twisted/blob/02a2b658cd1ade5d7f41f97d898913686313e615/src/twisted/protocols/basic.py#L857

Golang

The golang implementation also defines chunk size to be 16KB (recordSize := (1 << 14))

See https://github.com/psanford/wormhole-william/blob/68dc3447a8585b060fb1e6836a23847700ab9207/wormhole/send.go#L363

Rust

Rust is using 4KB.

https://github.com/magic-wormhole/magic-wormhole.rs/blob/6082d8b11d33b075285d31adc8a34ea03906f2cd/src/transfer/v1.rs#L585

I changed 4096 to 16384 and build a release build that gives me 117MB/s when sender is M1 pro Mac.

I think the rust implementation can also use 16KB.

HuakunShen avatar Apr 30 '24 00:04 HuakunShen

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 39.29%. Comparing base (6082d8b) to head (40c305b).

:exclamation: Current head 40c305b differs from pull request most recent head 619a366. Consider uploading reports for the commit 619a366 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #225      +/-   ##
==========================================
+ Coverage   39.24%   39.29%   +0.04%     
==========================================
  Files          18       18              
  Lines        3088     3092       +4     
==========================================
+ Hits         1212     1215       +3     
- Misses       1876     1877       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 30 '24 05:04 codecov[bot]