* [PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload
@ 2020-09-09 20:11 François Dumont
2020-09-10 15:05 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: François Dumont @ 2020-09-09 20:11 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 555 bytes --]
libstdc++: Use only public basic_streambuf methods in __copy_move_a2
overload
__copy_move_a2 for istreambuf_iterator can be implemented using public
basic_streambuf in_avail and sgetn so that __copy_move_a2 do not need to be
basic_streambuf friend.
libstdc++-v3/ChangeLog:
* include/std/streambuf (__copy_move_a2): Remove friend
declaration.
* include/bits/streambuf_iterator.h (__copy_move_a2):
Re-implement using
streambuf in_avail and sgetn.
Tested under Linux x86_64.
Ok to commit ?
François
[-- Attachment #2: copy_istreambuf_iterator.patch --]
[-- Type: text/x-patch, Size: 2168 bytes --]
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
index 184c82cd5bf..8712b90edd6 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -367,31 +367,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
istreambuf_iterator<_CharT> __last, _CharT* __result)
{
typedef istreambuf_iterator<_CharT> __is_iterator_type;
- typedef typename __is_iterator_type::traits_type traits_type;
typedef typename __is_iterator_type::streambuf_type streambuf_type;
- typedef typename traits_type::int_type int_type;
if (__first._M_sbuf && !__last._M_sbuf)
{
streambuf_type* __sb = __first._M_sbuf;
- int_type __c = __sb->sgetc();
- while (!traits_type::eq_int_type(__c, traits_type::eof()))
+ std::streamsize __avail = __sb->in_avail();
+ while (__avail > 0)
{
- const streamsize __n = __sb->egptr() - __sb->gptr();
- if (__n > 1)
- {
- traits_type::copy(__result, __sb->gptr(), __n);
- __sb->__safe_gbump(__n);
- __result += __n;
- __c = __sb->underflow();
- }
- else
- {
- *__result++ = traits_type::to_char_type(__c);
- __c = __sb->snextc();
- }
+ __result += __sb->sgetn(__result, __avail);
+ __avail = __sb->in_avail();
}
}
+
return __result;
}
diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
index cae35e75bda..13db284eb58 100644
--- a/libstdc++-v3/include/std/streambuf
+++ b/libstdc++-v3/include/std/streambuf
@@ -149,12 +149,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
friend streamsize
__copy_streambufs_eof<>(basic_streambuf*, basic_streambuf*, bool&);
- template<bool _IsMove, typename _CharT2>
- friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
- _CharT2*>::__type
- __copy_move_a2(istreambuf_iterator<_CharT2>,
- istreambuf_iterator<_CharT2>, _CharT2*);
-
template<typename _CharT2>
friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
istreambuf_iterator<_CharT2> >::__type
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload
2020-09-09 20:11 [PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload François Dumont
@ 2020-09-10 15:05 ` Jonathan Wakely
2020-09-10 19:29 ` François Dumont
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2020-09-10 15:05 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 09/09/20 22:11 +0200, François Dumont via Libstdc++ wrote:
>libstdc++: Use only public basic_streambuf methods in __copy_move_a2
>overload
>
>__copy_move_a2 for istreambuf_iterator can be implemented using public
>basic_streambuf in_avail and sgetn so that __copy_move_a2 do not need to be
>basic_streambuf friend.
>
>libstdc++-v3/ChangeLog:
>
>Â Â Â Â Â Â Â * include/std/streambuf (__copy_move_a2): Remove friend
>declaration.
>Â Â Â Â Â Â Â * include/bits/streambuf_iterator.h (__copy_move_a2):
>Re-implement using
>Â Â Â Â Â Â Â streambuf in_avail and sgetn.
Does this work?
The original code doesn't use any virtual functions except underflow,
which is called to refill the get area.
The new code calls in_avail() which is equivalent to the old
__sb->egptr() - __sb->gptr() code. If that returns a non-zero value
we'll use sgetn() to read that many characters (which makes a virtual
call to xsgetn). Then we call in_avail() again, which will now make a
virtual call to showmanyc(). There is no guarantee that showmanyc()
will return the correct value. It might return a conservative
underestimate, possibly even zero. If it returns zero then you break
out of the loop and return, but you haven't reached EOF.
Even if showmanyc() returns an accurate value (e.g. for filebuf it can
use a system call to find the size of the file on disk) you'll now
make another xsgetn() virtual call and showmanyc() virtual call for
each buffer's worth of characters. The original code only makes one
virtual call per buffer's worth of characters.
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>
>diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
>index 184c82cd5bf..8712b90edd6 100644
>--- a/libstdc++-v3/include/bits/streambuf_iterator.h
>+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
>@@ -367,31 +367,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> istreambuf_iterator<_CharT> __last, _CharT* __result)
> {
> typedef istreambuf_iterator<_CharT> __is_iterator_type;
>- typedef typename __is_iterator_type::traits_type traits_type;
> typedef typename __is_iterator_type::streambuf_type streambuf_type;
>- typedef typename traits_type::int_type int_type;
>
> if (__first._M_sbuf && !__last._M_sbuf)
> {
> streambuf_type* __sb = __first._M_sbuf;
>- int_type __c = __sb->sgetc();
>- while (!traits_type::eq_int_type(__c, traits_type::eof()))
>+ std::streamsize __avail = __sb->in_avail();
>+ while (__avail > 0)
> {
>- const streamsize __n = __sb->egptr() - __sb->gptr();
>- if (__n > 1)
>- {
>- traits_type::copy(__result, __sb->gptr(), __n);
>- __sb->__safe_gbump(__n);
>- __result += __n;
>- __c = __sb->underflow();
>- }
>- else
>- {
>- *__result++ = traits_type::to_char_type(__c);
>- __c = __sb->snextc();
>- }
>+ __result += __sb->sgetn(__result, __avail);
>+ __avail = __sb->in_avail();
> }
> }
>+
> return __result;
> }
>
>diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
>index cae35e75bda..13db284eb58 100644
>--- a/libstdc++-v3/include/std/streambuf
>+++ b/libstdc++-v3/include/std/streambuf
>@@ -149,12 +149,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> friend streamsize
> __copy_streambufs_eof<>(basic_streambuf*, basic_streambuf*, bool&);
>
>- template<bool _IsMove, typename _CharT2>
>- friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
>- _CharT2*>::__type
>- __copy_move_a2(istreambuf_iterator<_CharT2>,
>- istreambuf_iterator<_CharT2>, _CharT2*);
>-
> template<typename _CharT2>
> friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
> istreambuf_iterator<_CharT2> >::__type
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload
2020-09-10 15:05 ` Jonathan Wakely
@ 2020-09-10 19:29 ` François Dumont
0 siblings, 0 replies; 3+ messages in thread
From: François Dumont @ 2020-09-10 19:29 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 10/09/20 5:05 pm, Jonathan Wakely wrote:
> On 09/09/20 22:11 +0200, François Dumont via Libstdc++ wrote:
>> libstdc++: Use only public basic_streambuf methods in __copy_move_a2
>> overload
>>
>> __copy_move_a2 for istreambuf_iterator can be implemented using public
>> basic_streambuf in_avail and sgetn so that __copy_move_a2 do not need
>> to be
>> basic_streambuf friend.
>>
>> libstdc++-v3/ChangeLog:
>>
>> Â Â Â Â Â Â Â * include/std/streambuf (__copy_move_a2): Remove
>> friend declaration.
>> Â Â Â Â Â Â Â * include/bits/streambuf_iterator.h (__copy_move_a2):
>> Re-implement using
>> Â Â Â Â Â Â Â streambuf in_avail and sgetn.
>
> Does this work?
tests are passing yes, and there are a good number of it for this code.
>
> The original code doesn't use any virtual functions except underflow,
> which is called to refill the get area.
I see, I really didn't consider the virtual function calls. So I just
thought that less friend declaration and simpler code was better, my fault.
>
> The new code calls in_avail() which is equivalent to the old
> __sb->egptr() - __sb->gptr() code. If that returns a non-zero value
> we'll use sgetn() to read that many characters (which makes a virtual
> call to xsgetn). Then we call in_avail() again, which will now make a
> virtual call to showmanyc(). There is no guarantee that showmanyc()
> will return the correct value. It might return a conservative
> underestimate, possibly even zero. If it returns zero then you break
> out of the loop and return, but you haven't reached EOF.
I was concern about the 'If' in the showmanyc doc: If it returns a
positive value... But I think I forgot when I saw that tests were passing.
So, let's forget this patch.
Thanks for the feedback.
>
>
> Even if showmanyc() returns an accurate value (e.g. for filebuf it can
> use a system call to find the size of the file on disk) you'll now
> make another xsgetn() virtual call and showmanyc() virtual call for
> each buffer's worth of characters. The original code only makes one
> virtual call per buffer's worth of characters.
>
>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h
>> b/libstdc++-v3/include/bits/streambuf_iterator.h
>> index 184c82cd5bf..8712b90edd6 100644
>> --- a/libstdc++-v3/include/bits/streambuf_iterator.h
>> +++ b/libstdc++-v3/include/bits/streambuf_iterator.h
>> @@ -367,31 +367,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> istreambuf_iterator<_CharT> __last, _CharT* __result)
>> {
>> typedef istreambuf_iterator<_CharT> __is_iterator_type;
>> - typedef typename __is_iterator_type::traits_type traits_type;
>> typedef typename __is_iterator_type::streambuf_type
>> streambuf_type;
>> - typedef typename traits_type::int_type int_type;
>>
>> if (__first._M_sbuf && !__last._M_sbuf)
>> {
>> streambuf_type* __sb = __first._M_sbuf;
>> - int_type __c = __sb->sgetc();
>> - while (!traits_type::eq_int_type(__c, traits_type::eof()))
>> + std::streamsize __avail = __sb->in_avail();
>> + while (__avail > 0)
>> {
>> - const streamsize __n = __sb->egptr() - __sb->gptr();
>> - if (__n > 1)
>> - {
>> - traits_type::copy(__result, __sb->gptr(), __n);
>> - __sb->__safe_gbump(__n);
>> - __result += __n;
>> - __c = __sb->underflow();
>> - }
>> - else
>> - {
>> - *__result++ = traits_type::to_char_type(__c);
>> - __c = __sb->snextc();
>> - }
>> + __result += __sb->sgetn(__result, __avail);
>> + __avail = __sb->in_avail();
>> }
>> }
>> +
>> return __result;
>> }
>>
>> diff --git a/libstdc++-v3/include/std/streambuf
>> b/libstdc++-v3/include/std/streambuf
>> index cae35e75bda..13db284eb58 100644
>> --- a/libstdc++-v3/include/std/streambuf
>> +++ b/libstdc++-v3/include/std/streambuf
>> @@ -149,12 +149,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> friend streamsize
>> __copy_streambufs_eof<>(basic_streambuf*, basic_streambuf*,
>> bool&);
>>
>> - template<bool _IsMove, typename _CharT2>
>> - friend typename
>> __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
>> - _CharT2*>::__type
>> - __copy_move_a2(istreambuf_iterator<_CharT2>,
>> - istreambuf_iterator<_CharT2>, _CharT2*);
>> -
>> template<typename _CharT2>
>> friend typename
>> __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
>> istreambuf_iterator<_CharT2> >::__type
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-10 19:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 20:11 [PATH 1/3] libstdc++: Simplify std::copy istreambuf_iterator overload François Dumont
2020-09-10 15:05 ` Jonathan Wakely
2020-09-10 19:29 ` François Dumont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).