Skip to content

feat(zmodem): accelerate upload and download transfers - #3147

Merged
binaricat merged 7 commits into
binaricat:mainfrom
lswzw:feat/zmodem-transfer-acceleration
Aug 28, 2026
Merged

feat(zmodem): accelerate upload and download transfers#3147
binaricat merged 7 commits into
binaricat:mainfrom
lswzw:feat/zmodem-transfer-acceleration

Conversation

@lswzw

@lswzw lswzw commented Aug 25, 2026

Copy link
Copy Markdown

直接粘贴以下内容:

## Summary

Implement ZMODEM upload and download acceleration for Netcatty by reducing per-byte array processing and payload allocations.

## Type of Change

- [ ] Bug fix
- [x] New feature
- [ ] Refactor / code cleanup
- [ ] Documentation update
- [ ] Build / CI change
- [ ] Other (please describe):

## Related Issue (optional)

N/A

## Changes Made

- Add Buffer/Uint8Array fast paths for ZMODEM receive and send data.
- Add typed-array CRC16/CRC32 processing and zero-copy download payload handling.
- Reduce upload allocation overhead with direct frame encoding and larger read chunks.
- Throttle upload progress events across transports.
- Improve ZMODEM send-session keepalive and handshake handling.
- Add `NETCATTY_ZMODEM_FAST_PATH=0` to disable both performance paths for troubleshooting.
- Add receive-path regression and protocol-equivalence tests.

## Screenshots / Demo

N/A - terminal transfer performance change.

## Testing

- [ ] I have tested these changes locally (`npm run dev`) (not applicable; no UI changes)
- [x] Linting passes (`npm run lint`)
- [ ] Tests pass (`npm test`) (targeted tests pass; full suite is blocked by the existing unmatched glob `electron/capabilities/*/*/*.test.cjs`)
- [x] Generated capability tool specs are updated when applicable (not applicable)
- [x] No new console errors or warnings, if this affects app behavior

Additional verification:

- Targeted ZMODEM and bridge tests: 4/4 passed.
- Receive parser benchmark: approximately 101 MB/s on local test data.
- `node --check` and staged diff checks passed.

## Checklist

- [x] My code follows the existing project style
- [x] I have added or updated relevant documentation (inline implementation comments)
- [x] I have not introduced any breaking changes (the fast receive payload is intentionally `Uint8Array` internally for zero-copy handling)

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1768e8d993

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread electron/bridges/zmodemHelper.cjs Outdated
// (drain check + yield + progress IPC), so larger chunks amortize that
// overhead across many more bytes. The wire subpackets stay at the
// library's 8192-byte MAX_CHUNK_LENGTH.
const UPLOAD_CHUNK_SIZE = 1024 * 1024;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve bounded chunks when no drain waiter exists

Keep the smaller upload granularity for transports without a real drain implementation. I checked the supported first-party local PTY, ET, Mosh, Telnet, and serial registrations, and none supplies waitForTransportDrain; when their writeToRemote() returns false, createZmodemUploadDrainWaiter() merely clears _needsDrain after one setImmediate. Because xfer.send() synchronously queues the entire chunk before that check, this change now queues another 1 MiB on every event-loop turn regardless of whether the slow writable has drained, allowing large uploads to grow the process buffer toward the file size and potentially exhaust memory or lose the session.

Useful? React with 👍 / 👎.

@binaricat

Copy link
Copy Markdown
Owner

@codex review

1 similar comment
@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 956f6c53f6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: ba6d832ee4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4bbb7468f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread electron/bridges/zmodemFastPath.cjs
@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: bd1aa604f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@binaricat
binaricat merged commit 7181486 into binaricat:main Aug 28, 2026
27 checks passed
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