Page MenuHomePhabricator

Partially inline basic_string copy constructor in UNSTABLE
AbandonedPublic

Authored by mvels on Mar 4 2020, 12:50 PM.

Details

Summary

This is a recommit of https://reviews.llvm.org/D73223 where the added function accidentally ended up inside an idef block.

This change splits the copy constructor up inlining short initialization, and explicitly outlining long initialization into __init_copy_ctor_external() which is the externally instantiated slow path.

For unstable ABI, this has the following changes:

remove basic_string(const basic_string&)
remove basic_string(const basic_string&, const Allocator&)
add __init_copy_ctor_external(const value_type*, size_type)
Quick local benchmark for Copy:

Master

---------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
---------------------------------------------------------------
BM_StringCopy_Empty       3.50 ns         3.51 ns    199326720
BM_StringCopy_Small       3.50 ns         3.51 ns    199510016
BM_StringCopy_Large       15.7 ns         15.7 ns     45230080
BM_StringCopy_Huge        1503 ns         1503 ns       464896

With this change

---------------------------------------------------------------
Benchmark                    Time             CPU   Iterations
---------------------------------------------------------------
BM_StringCopy_Empty       1.99 ns         2.00 ns    356471808
BM_StringCopy_Small       3.29 ns         3.30 ns    203425792
BM_StringCopy_Large       13.3 ns         13.3 ns     52948992
BM_StringCopy_Huge        1472 ns         1472 ns       475136

Diff Detail

Event Timeline

mvels created this revision.Mar 4 2020, 12:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2020, 12:50 PM
EricWF accepted this revision.Mar 4 2020, 2:18 PM

This diff is incorrect, but I see it contains the fix to the previous failures. So it LGTM assuming you apply the correct diff.

This revision is now accepted and ready to land.Mar 4 2020, 2:18 PM
mvels updated this revision to Diff 248315.Mar 4 2020, 2:18 PM

Diff against

mvels added a comment.Mar 4 2020, 2:48 PM

Thanks, arc is not my friend :\ Fixed the diff using --base=git:origin

(one could argue 'arc' was smart to recognize this was a rolled forward revert, and only diff against the original submit, alas, not my desired form of smart)

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 27 2020, 12:53 PM
This revision now requires review to proceed.May 27 2020, 12:53 PM
mvels abandoned this revision.May 27 2020, 12:55 PM