[WIP] backend/external: improve sort speed for external writer

[joechenrh]

What problem does this PR solve?

Issue Number: close #xxx

This PR is used to do some tests.

This PR enlarge the size of SliceLocation by 33%(12 bytes -> 16 bytes), which may increase memory usage of each subtask by 77MiB at most (for CPU/Mem = 1C/2GiB). Considering that memory usage of read step is an issue, this PR is just used to illustrate the cost of getting byte slice during sort.

Problem Summary:

What changed and how does it work?

Encapsulate 4 bytes key hint in SliceLocation to speed up sorting without enlarging the memory usage.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Below are the manual test of encode(read) speed

Add Index

• row count:
• 8B random string: 00:12:15 -> 00:08:29
• 120B random string: 00:13:15 -> 00:08:41

We also test some worst case for this PR, to verify that there are no degradation.

Import Into with index

• Table size: 500GiB
• 01:00:54 -> 00:55:20

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

[tiprow]

Hi @joechenrh. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[Copilot]

Pull request overview

This PR refactors the byteSet implementation in the CSV parser from a value type using bitsets to a pointer-based type using a lookup table. The changes convert the byteSet from an array type to a struct containing both a bit array and a byte lookup table.

Key Changes:

• Refactored byteSet from array type to struct with bits and table fields
• Changed makeByteSet to return a pointer and initialize the lookup table
• Updated CSV parser to use pointer types for byte sets and removed reference operators in function calls

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
pkg/lightning/mydump/bytes.go	Refactored byteSet from array to struct with lookup table, modified makeByteSet to return pointer, removed contains method, updated IndexAnyByte to use table lookup
pkg/lightning/mydump/csv_parser.go	Changed byteSet field types from value to pointer, updated readUntil calls to pass pointers directly instead of taking references

[Copilot]

The table index should be c (the byte value), not i (the loop index). Currently this sets table[0], table[1], etc., instead of setting the correct positions for the actual byte values in the input. This will cause incorrect behavior when checking if a byte is in the set.

The line should be: as.table[c] = 1

Suggested change

	as.table[i] = 1
	as.table[c] = 1

[Copilot]

The bits field is no longer used after the refactoring to use the table lookup approach. This field should either be removed or the bit-based lookup should be used instead of the table. Keeping unused fields increases memory overhead (32 bytes per byteSet instance) and may confuse future maintainers about the intended implementation.

[Copilot]

The documentation comment is now inaccurate. The byteSet is no longer a "32-byte value" - it's now a struct containing a 32-byte bits array and a 256-byte table array (288 bytes total). The comment should be updated to reflect the actual structure, or explain the purpose of the lookup table approach if that's the intended design.

Suggested change

	// byteSet is a 32-byte value, where each bit represents the presence of a
	// given byte value in the set.
	// byteSet represents a set of byte values. It uses a 32-byte bitset (bits)
	// where each bit indicates the presence of a given byte value, and a 256-byte
	// lookup table (table) for O(1) membership checks by byte value.

[codecov]

Codecov Report

❌ Patch coverage is 1.51515% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.9148%. Comparing base (c6b0d30) to head (a3c066e).
⚠️ Report is 39 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #65137        +/-   ##
================================================
+ Coverage   77.8425%   78.9148%   +1.0722%     
================================================
  Files          1972       1915        -57     
  Lines        541074     539550      -1524     
================================================
+ Hits         421186     425785      +4599     
+ Misses       118229     111551      -6678     
- Partials       1659       2214       +555     

Flag	Coverage Δ
integration	47.6098% <1.5151%> (-0.5867%)	⬇️

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

Components	Coverage Δ
dumpling	56.7974% <ø> (ø)
parser	∅ <ø> (∅)
br	66.1568% <ø> (+5.0093%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ti-chi-bot]

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-linked-issue label, please provide the linked issue number on one line in the PR body, for example: Issue Number: close #123 or Issue Number: ref #456.

_{📖 For more info, you can check the "Contribute Code" section in the development guide.}

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gmhdbjd for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• pkg/ddl/OWNERS
• pkg/lightning/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

@joechenrh: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-unit-test-next-gen	a3c066e	link	true	/test pull-unit-test-next-gen
idc-jenkins-ci-tidb/unit-test	a3c066e	link	true	/test unit-test
pull-unit-test-ddlv1	a3c066e	link	true	/test pull-unit-test-ddlv1

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.