Skip to content

perf: Use Cow to avoid creates String on format. #248

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

Merged
merged 3 commits into from
Mar 23, 2025

Conversation

quake
Copy link
Contributor

@quake quake commented Mar 23, 2025

This pull request focuses on improving the performance and memory usage of the formatting functions by changing their return types to Cow<str>. This allows the functions to return either borrowed or owned data, reducing unnecessary allocations when the input is not modified.

Most benchmarks gain 10% ~ 20% improvement on my local PC:

format_json             time:   [171.73 µs 172.00 µs 172.28 µs]
                        change: [-17.792% -17.409% -17.049%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

format_javascript       time:   [360.43 µs 360.88 µs 361.32 µs]
                        change: [-15.179% -13.906% -12.966%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high severe

format_json_2k          time:   [21.865 ms 21.882 ms 21.900 ms]
                        change: [-15.133% -14.999% -14.869%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

format_jupyter          time:   [275.67 µs 276.03 µs 276.38 µs]
                        change: [-7.5263% -7.2771% -7.0001%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  4 (4.00%) high mild
  3 (3.00%) high severe

format_markdown         time:   [2.2873 ms 2.2896 ms 2.2923 ms]
                        change: [-14.583% -14.448% -14.325%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

lint_markdown           time:   [2.3867 ms 2.3939 ms 2.4021 ms]
                        change: [-15.337% -15.092% -14.799%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe

lint_json               time:   [180.12 µs 180.38 µs 180.64 µs]
                        change: [-19.382% -18.912% -18.449%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe

lint_html               time:   [694.02 µs 694.96 µs 695.95 µs]
                        change: [-15.000% -14.666% -14.362%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe

lint_javascript         time:   [367.04 µs 367.55 µs 368.05 µs]
                        change: [-27.115% -23.420% -20.369%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)

@huacnlee huacnlee changed the title perf: use Cow perf: Use Cow to avoid creates String on format. Mar 23, 2025
@huacnlee
Copy link
Owner

我早就想优化这部分了,里面有太多的 format 不断创建新的 String。

Cow 正好解决这个问题,每次应该只有少量的情况有修改。

}

/// Normalize chars to use general half width in Chinese contents.
Copy link
Owner

Choose a reason for hiding this comment

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

这行注释弄掉了,看起来像 AI 干的 😄

} else {
result.severity = Severity::Error;
if let Cow::Owned(new) = (self.format_fn)(&result.out) {
if result.severity == Severity::Pass {
Copy link
Owner

Choose a reason for hiding this comment

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

Missed changed check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is inside the Cow::Owned block, which means it's changed already.

@@ -35,27 +37,27 @@ impl Rule {
return;
}

let new = (self.format_fn)(&result.out);
if result.out.ne(&new) {
Copy link
Owner

Choose a reason for hiding this comment

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

Missed changed check here.

@huacnlee huacnlee merged commit d6ea7c1 into huacnlee:main Mar 23, 2025
6 checks passed
@huacnlee
Copy link
Owner

棒!👍👍👍

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