Commit 853d5b59 authored by Marcin Pawlowski's avatar Marcin Pawlowski Committed by Andrii Grynenko

stop prereserving space in toAppend

Summary:
previous change would cause reserving
as much as needed in toAppend for all arguements.
This had bad consequences for appending in a loop,
described here: https://phabricator.fb.com/D1420588#22
Not split the interfaces so that user has to decide

Test Plan: unit tests

Reviewed By: soren@fb.com

Subscribers: trunkagent, njormrod

FB internal diff: D1601614

Tasks: 5303991
parent b692cb2f
...@@ -717,8 +717,10 @@ toAppendDelimStrImpl(const Delimiter& delim, const T& v, const Ts&... vs) { ...@@ -717,8 +717,10 @@ toAppendDelimStrImpl(const Delimiter& delim, const T& v, const Ts&... vs) {
/** /**
* Variadic conversion to string. Appends each element in turn. * Variadic conversion to string. Appends each element in turn.
* If we have two or more things to append, we will reserve * If we have two or more things to append, we it will not reserve
* the space for them (at least we will try). * the space for them and will depend on strings exponential growth.
* If you just append once consider using toAppendFit which reserves
* the space needed (but does not have exponential as a result).
*/ */
template <class... Ts> template <class... Ts>
typename std::enable_if<sizeof...(Ts) >= 3 typename std::enable_if<sizeof...(Ts) >= 3
...@@ -727,10 +729,32 @@ typename std::enable_if<sizeof...(Ts) >= 3 ...@@ -727,10 +729,32 @@ typename std::enable_if<sizeof...(Ts) >= 3
typename detail::last_element<Ts...>::type typename detail::last_element<Ts...>::type
>::type>::value>::type >::type>::value>::type
toAppend(const Ts&... vs) { toAppend(const Ts&... vs) {
detail::reserveInTarget(vs...);
detail::toAppendStrImpl(vs...); detail::toAppendStrImpl(vs...);
} }
/**
* Special version of the call that preallocates exaclty as much memory
* as need for arguments to be stored in target. This means we are
* not doing exponential growth when we append. If you are using it
* in a loop you are aiming at your foot with a big perf-destroying
* bazooka.
* On the other hand if you are appending to a string once, this
* will probably save a few calls to malloc.
*/
template <class... Ts>
typename std::enable_if<
IsSomeString<
typename std::remove_pointer<
typename detail::last_element<Ts...>::type
>::type>::value>::type
toAppendFit(const Ts&... vs) {
detail::reserveInTarget(vs...);
toAppend(vs...);
}
template <class Ts>
void toAppendFit(const Ts&) {}
/** /**
* Variadic base case: do nothing. * Variadic base case: do nothing.
*/ */
...@@ -757,7 +781,8 @@ toAppendDelim(const Delimiter& delim, const T& v, Tgt* tgt) { ...@@ -757,7 +781,8 @@ toAppendDelim(const Delimiter& delim, const T& v, Tgt* tgt) {
} }
/** /**
* Append to string with a delimiter in between elements. * Append to string with a delimiter in between elements. Check out
* comments for toAppend for details about memory allocation.
*/ */
template <class Delimiter, class... Ts> template <class Delimiter, class... Ts>
typename std::enable_if<sizeof...(Ts) >= 3 typename std::enable_if<sizeof...(Ts) >= 3
...@@ -766,10 +791,26 @@ typename std::enable_if<sizeof...(Ts) >= 3 ...@@ -766,10 +791,26 @@ typename std::enable_if<sizeof...(Ts) >= 3
typename detail::last_element<Ts...>::type typename detail::last_element<Ts...>::type
>::type>::value>::type >::type>::value>::type
toAppendDelim(const Delimiter& delim, const Ts&... vs) { toAppendDelim(const Delimiter& delim, const Ts&... vs) {
detail::reserveInTargetDelim(delim, vs...);
detail::toAppendDelimStrImpl(delim, vs...); detail::toAppendDelimStrImpl(delim, vs...);
} }
/**
* Detail in comment for toAppendFit
*/
template <class Delimiter, class... Ts>
typename std::enable_if<
IsSomeString<
typename std::remove_pointer<
typename detail::last_element<Ts...>::type
>::type>::value>::type
toAppendDelimFit(const Delimiter& delim, const Ts&... vs) {
detail::reserveInTargetDelim(delim, vs...);
toAppendDelim(delim, vs...);
}
template <class De, class Ts>
void toAppendDelimFit(const De&, const Ts&) {}
/** /**
* to<SomeString>(SomeString str) returns itself. As both std::string and * to<SomeString>(SomeString str) returns itself. As both std::string and
* folly::fbstring use Copy-on-Write, it's much more efficient by * folly::fbstring use Copy-on-Write, it's much more efficient by
...@@ -795,7 +836,7 @@ typename std::enable_if< ...@@ -795,7 +836,7 @@ typename std::enable_if<
Tgt>::type Tgt>::type
to(const Ts&... vs) { to(const Ts&... vs) {
Tgt result; Tgt result;
toAppend(vs..., &result); toAppendFit(vs..., &result);
return result; return result;
} }
...@@ -822,7 +863,7 @@ typename std::enable_if< ...@@ -822,7 +863,7 @@ typename std::enable_if<
Tgt>::type Tgt>::type
toDelim(const Delim& delim, const Ts&... vs) { toDelim(const Delim& delim, const Ts&... vs) {
Tgt result; Tgt result;
toAppendDelim(delim, vs..., &result); toAppendDelimFit(delim, vs..., &result);
return result; return result;
} }
......
...@@ -748,6 +748,22 @@ TEST(Conv, NewUint64ToString) { ...@@ -748,6 +748,22 @@ TEST(Conv, NewUint64ToString) {
#undef THE_GREAT_EXPECTATIONS #undef THE_GREAT_EXPECTATIONS
} }
TEST(Conv, allocate_size) {
std::string str1 = "meh meh meh";
std::string str2 = "zdech zdech zdech";
auto res1 = folly::to<std::string>(str1, ".", str2);
EXPECT_EQ(res1, str1 + "." + str2);
std::string res2; //empty
toAppendFit(str1, str2, 1, &res2);
EXPECT_EQ(res2, str1 + str2 + "1");
std::string res3;
toAppendDelimFit(",", str1, str2, &res3);
EXPECT_EQ(res3, str1 + "," + str2);
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// Benchmarks for ASCII to int conversion // Benchmarks for ASCII to int conversion
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment