Skip to content

fix(codec): support copying thrift fast writes (similar to pr 1983 ) - #1988

Open
wdp2020 wants to merge 2 commits into
cloudwego:mainfrom
wdp2020:fix/fast-write-copy
Open

fix(codec): support copying thrift fast writes (similar to pr 1983 )#1988
wdp2020 wants to merge 2 commits into
cloudwego:mainfrom
wdp2020:fix/fast-write-copy

Conversation

@wdp2020

@wdp2020 wdp2020 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

Description
The Thrift fast-write path currently passes the output buffer as a remote.NocopyWrite implementation to generated FastWriteNocopy methods. For string and binary fields, this causes the generated code to use WriteDirect, followed by MallocAck, instead of copying the data into the buffer allocated for the encoded message.
In workloads containing large string or binary fields, the WriteDirect and MallocAck path can introduce significant CPU and buffer-management overhead. In the observed profile, response encoding consumed 53.02% of CPU time, with two large-string WriteDirect paths accounting for approximately 49.45%.
This change adds NewThriftCodecWithFastWriteCopy, an opt-in codec constructor that keeps Thrift fast read/write enabled while forcing string and binary fields to be copied into the preallocated output buffer.
When copy mode is enabled, encodeFastThrift passes a nil NocopyWrite writer to FastWriteNocopy. The generated code therefore writes those fields into the buffer returned by Malloc instead of calling WriteDirect and MallocAck.
The option is applied consistently to:
Normal fast Thrift encoding
Fast-write fallback encoding
Thrift exception encoding

Why It Is Necessary
The existing codec does not provide a way to disable direct writes without also giving up the generated fast codec path. Although direct writes can reduce copying for some workloads, they can be more expensive when handling large fields with transports whose direct-write implementation requires additional buffer operations.
Making this behavior opt-in provides a targeted workaround while preserving the existing default behavior for all current users.

Compatibility
This change is backward compatible:
NewThriftCodec() retains the existing no-copy behavior.
Copy mode is enabled only through NewThriftCodecWithFastWriteCopy().
The Thrift wire format is unchanged.
Existing codec configuration and generated code remain compatible.
Tests verify that the default codec still calls WriteDirect and MallocAck, while the new codec avoids both operations and produces payloads that can be decoded correctly.

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:
zh(optional):

(Optional) Which issue(s) this PR fixes:

(optional) The PR that updates user documentation:

@wdp2020
wdp2020 requested review from a team as code owners August 19, 2026 10:13
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.02%. Comparing base (8bb270e) to head (0df0573).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1988      +/-   ##
==========================================
+ Coverage   62.95%   63.02%   +0.06%     
==========================================
  Files         394      396       +2     
  Lines       30267    30351      +84     
==========================================
+ Hits        19056    19129      +73     
- Misses       9924     9937      +13     
+ Partials     1287     1285       -2     
Flag Coverage Δ
integration 51.94% <70.00%> (+0.06%) ⬆️
unit 53.63% <100.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant