Add string builder to build `JsString`
There are many places where Vec is used as a builder to create JsString.
This adds JsStringBuilder as a more natural choice.
Codecov Report
Attention: Patch coverage is 67.68061% with 85 lines in your changes missing coverage. Please review.
Project coverage is 52.96%. Comparing base (
6ddc2b4) to head (bb27bd7). Report is 298 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| core/string/src/builder.rs | 67.68% | 85 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #3915 +/- ##
==========================================
+ Coverage 47.24% 52.96% +5.72%
==========================================
Files 476 484 +8
Lines 46892 47210 +318
==========================================
+ Hits 22154 25007 +2853
+ Misses 24738 22203 -2535
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚨 Try these New Features:
- Flaky Tests Detection - Detect and resolve failed and flaky tests
@HalidOdat, one more thing is that I used realloc instead of alloc before and found alloc has a better performance, since I can't test on other machine, you could test realloc to see if I'm wrong.
If you want to test it, just change the reserve:
fn reserve(&mut self, new_layout: Layout) {
let new_ptr = if self.is_dangling() {
let new_ptr = unsafe { alloc(new_layout) };
let Some(new_ptr) = NonNull::new(new_ptr.cast::<RawJsString>()) else {
std::alloc::handle_alloc_error(new_layout)
};
new_ptr
} else {
use std::alloc::realloc;
let old_ptr = self.inner.as_ptr();
let ptr = unsafe { realloc(ptr, new_layout, new_layout.size()) };
let Some(new_ptr) = NonNull::new(new_ptr.cast::<RawJsString>()) else {
std::alloc::handle_alloc_error(new_layout)
};
new_ptr
};
self.inner = new_ptr;
let new_arr_size = new_layout.size() - DATA_OFFSET;
self.cap = new_arr_size / Self::DATA_SIZE;
}
let ptr = unsafe { realloc(ptr, new_layout, new_layout.size()) };
This may be unsafe because in the realloc's safety section it has the requirement that the layout passed to it is layout must be the same layout that was used to allocate that block of memory, so it should be the current layout.
See: https://doc.rust-lang.org/stable/core/alloc/trait.GlobalAlloc.html#method.realloc
I benchmarked with the current implementation (alloc) and with realloc and found that it's considerably faster (14x), here is my benchmark code if you want to reproduce it.
fn main() {
let mut sb = JsStringBuilder::<u8>::new();
for _ in 0..10000 {
sb.push(b'a');
sb.extend_from_slice(b"bcde".as_slice());
sb.extend([b'f', b'g'].into_iter());
}
let s = sb.build();
drop(s);
}
hyperfine -N --warmup=10 ./sb-alloc ./sb-realloc
Benchmark 1: ./sb-alloc
Time (mean ± σ): 10.8 ms ± 0.2 ms [User: 5.9 ms, System: 4.6 ms]
Range (min … max): 10.2 ms … 11.9 ms 284 runs
Benchmark 2: ./sb-realloc
Time (mean ± σ): 728.6 µs ± 78.9 µs [User: 565.7 µs, System: 96.5 µs]
Range (min … max): 550.7 µs … 1324.8 µs 3970 runs
Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
Summary
./sb-realloc ran
14.77 ± 1.63 times faster than ./sb-alloc
NOTE: Changed extend's reserve to be self.reserve(Self::new_layout(self.len() + lower_bound)); to avoid the above mentioned bug.
@HalidOdat
My OS is Windows11, and seems realloc doesn't work much better for me, what is your OS?
And do you think I should change into realloc?
Benchmark 1: ./jsstringbuilder-alloc.exe
Time (mean ± σ): 3.2 ms ± 0.2 ms [User: 0.7 ms, System: 1.9 ms]
Range (min … max): 3.0 ms … 6.0 ms 952 runs
Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
Benchmark 2: ./jsstringbuilder-realloc.exe
Time (mean ± σ): 3.0 ms ± 0.2 ms [User: 1.0 ms, System: 1.6 ms]
Range (min … max): 2.8 ms … 6.1 ms 1028 runs
Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
Summary
./jsstringbuilder-realloc.exe ran
1.07 ± 0.08 times faster than ./jsstringbuilder-alloc.exe
My OS is Windows11, and seems realloc doesn't work much better for me, what is your OS?
I use Linux (EndeavourOS).
And do you think I should change into realloc?
Yes, there is no drawback to using realloc and even on windows (from the results you posted) there is still a small ~7% increase in performance.
Yes, there is no drawback to using realloc and even on windows (from the results you posted) there is still a small
~7%increase in performance.
Ok