Fix copy_str performance 2 #2477
Conversation
Could you elaborate? |
|
I'll reopen #2475 and close this one since all the discussion is there. |
|
Actually, I cannot reopen #2475 because the master branch was force-pushed. |
Roman-Koshelev
force-pushed
the
master
branch
from
c9eef45
to
dd022b1
Compare
include/fmt/core.h
Outdated
| return copy_str_constexpr<Char>(begin, end, out); | ||
| } | ||
|
|
||
| template <typename Char, typename InputIt, typename OutputIt, |
There was a problem hiding this comment.
This is probably nitpicky, but since InputIt and OutputIt aren't iterators in this overload, can you rename the template parameters? I keep getting concerned when I see the "* sizeof(OutputIt)" in the call to memcpy below.
Roman-Koshelev
force-pushed
the
master
branch
from
dd022b1
to
10905c8
Compare
|
I understand that we have reached an implementation that suits both of us? |
include/fmt/core.h
Outdated
| FMT_CONSTEXPR auto copy_str_constexpr(InputIt begin, InputIt end, OutputIt out) | ||
| -> OutputIt { | ||
| while (begin != end) *out++ = static_cast<Char>(*begin++); | ||
| return out; | ||
| } |
There was a problem hiding this comment.
I suggest merging this into the first overload of copy_str as before. The second overload can continue calling it as copy_str<Char, const Char*, Char*>(...).
There was a problem hiding this comment.
I thought it would lead to recursion, but it doesn't
include/fmt/core.h
Outdated
| return copy_str_constexpr<Char>(begin, end, out); | ||
| } | ||
|
|
||
| template <typename Char, typename Tp, typename Up, |
There was a problem hiding this comment.
nit: Tp -> T, Up -> U for consistency with naming conventions in the library.
Roman-Koshelev
force-pushed
the
master
branch
2 times, most recently
from
5825d11
to
91526ae
Compare
Roman-Koshelev
force-pushed
the
master
branch
from
91526ae
to
a20dba6
Compare
|
Thank you! |
Contrary to expectations, optimized overloading is not called