Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #10121 > unrolled thread

Thoughts on function

Back to article view | Back to comp.lang.c++

Contents

  Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-14 22:01 -0700
    Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-15 17:08 +1200
      Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-15 07:34 -0700
        Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-16 07:38 +1200
          Re: Thoughts on function Leigh Johnston <leigh@i42.co.uk> - 2011-09-15 20:40 +0100
          Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-15 17:23 -0700
            Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-18 13:30 -0700
              Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-19 09:34 +1200
                Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-18 18:25 -0700
                  Re: Thoughts on function Geoff <geoff@invalid.invalid> - 2011-09-18 19:08 -0700
                    Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-18 19:46 -0700
                      Re: Thoughts on function Geoff <geoff@invalid.invalid> - 2011-09-18 20:12 -0700
                        Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-18 21:28 -0700
                    Re: Thoughts on function Miles Bader <miles@gnu.org> - 2011-09-19 13:26 +0900
                      Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-19 16:45 +1200
                        Re: Thoughts on function Miles Bader <miles@gnu.org> - 2011-09-19 15:19 +0900
                  Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-19 16:43 +1200
                    Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-19 11:16 -0700
                      Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-20 07:44 +1200
                        Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-19 18:03 -0700
                          Re: Thoughts on function Ian Collins <ian-news@hotmail.com> - 2011-09-20 13:15 +1200
                            Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-19 18:27 -0700
    Re: Thoughts on function Fulvio Esposito <esposito.fulvio@gmail.com> - 2011-09-16 02:19 -0700
      Re: Thoughts on function Ebenezer <woodbrian77@gmail.com> - 2011-09-16 11:20 -0700
        Re: Thoughts on function Fulvio Esposito <esposito.fulvio@gmail.com> - 2011-09-16 13:51 -0700
    Re: Thoughts on function hanukas <jukka@liimatta.org> - 2011-09-19 06:19 -0700

Page 2 of 2 — ← Prev page 1 [2]

#10309

On 09/20/11 01:03 PM, Ebenezer wrote:
> On Sep 19, 2:44 pm, Ian Collins<ian-n...@hotmail.com>  wrote:
>>
>> Well you've already made partialFlush static, so it's a obvious
>> contender for a a member variable.  The other two could be parameters.
>>
>
> bool
> SendBuffer::ending (uint32_t all, uint32_t numWritten)

What is your objection to size_t?

<snip>

> The .o file is 48 bytes bigger in this form when using
> g++ 4.5.1 and -O3.  I'm not sure this is a step in the
> right direction.

Are you programming for a PIC?  Otherwise why worry about 48 bytes?

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#10310

On Sep 19, 8:15 pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 09/20/11 01:03 PM, Ebenezer wrote:
>
> > On Sep 19, 2:44 pm, Ian Collins<ian-n...@hotmail.com>  wrote:
>
> >> Well you've already made partialFlush static, so it's a obvious
> >> contender for a a member variable.  The other two could be parameters.
>
> > bool
> > SendBuffer::ending (uint32_t all, uint32_t numWritten)
>
> What is your objection to size_t?

None really.  I may change it.

>
> <snip>
>
> > The .o file is 48 bytes bigger in this form when using
> > g++ 4.5.1 and -O3.  I'm not sure this is a step in the
> > right direction.
>
> Are you programming for a PIC?  Otherwise why worry about 48 bytes?
>

I'm not worried about it, but am looking for some clues
as to whether that form is a step in the right direction.
The 48 additional bytes makes me wonder a little.

[toc] | [prev] | [next] | [standalone]

#10193

Hi Brian,
I'm not really an expert, but here a couple of suggestion

> bool
> SendBuffer::Flush ()
> {
>   uint32_t const all = buf_.size();
>   uint32_t numWritten = 0;

use std::size_t instead of uint32_t, it's more portable!

>   if (partialFlush_) {
>     ::neolib::segmented_array<unsigned char, chunk_size>::segment&
> segment =
>                segmented_iterator<unsigned char,
> chunk_size>(buf_.begin()).segment();
>
>     numWritten = Write(sock_, &buf_[0], segment.size());
>     if (numWritten < segment.size()) goto ending;
>     partialFlush_ = false;
>   }

is partialFlush_ a flag about last flush operation and not the
current? That is, if a flush doesn't complete for any reason, you set
partial flush to true, and the next time flush is called, you send the
pending bytes (in fact, the entire segment that contains the first
byte in the buffer), am I right?
Maybe I can't see how useful this is 'cause I have not read all the
code, but if flush simply try to send the entire buffer, and if it
cannot, simply erase only the sended part and leave the rest to the
next call?

>
>   {
>     int32_t const chunks = (all - numWritten) / chunk_size;
>     for (int32_t ii = 0; ii < chunks; ++ii) {
>       uint32_t bytes = Write(sock_, &buf_[numWritten], chunk_size);
>       numWritten += bytes;
>       if (bytes < chunk_size) goto ending;
>     }
>   }

Maybe the outer enclosing clock could be eliminated, it's not useful
here.



>
>   if (numWritten < all) {
>     numWritten += Write(sock_, &buf_[numWritten], all - numWritten);
>   }
>
> ending:
>   buf_.erase(buf_.begin(), buf_.begin() + numWritten);
>   if (numWritten == all) return true;
>
>   partialFlush_ = true;
>   return false;
>
> }

goto is considered worst practice :) maybe a redesign to eliminate it
could be better.

Bye,
Fulvio Esposito

[toc] | [prev] | [next] | [standalone]

#10197

On Sep 16, 4:19 am, Fulvio Esposito <esposito.ful...@gmail.com> wrote:
> Hi Brian,
> I'm not really an expert, but here a couple of suggestion
>
> > bool
> > SendBuffer::Flush ()
> > {
> >   uint32_t const all = buf_.size();
> >   uint32_t numWritten = 0;
>
> use std::size_t instead of uint32_t, it's more portable!
>
> >   if (partialFlush_) {
> >     ::neolib::segmented_array<unsigned char, chunk_size>::segment&
> > segment =
> >                segmented_iterator<unsigned char,
> > chunk_size>(buf_.begin()).segment();
>
> >     numWritten = Write(sock_, &buf_[0], segment.size());
> >     if (numWritten < segment.size()) goto ending;
> >     partialFlush_ = false;
> >   }
>
> is partialFlush_ a flag about last flush operation and not the
> current?

Yes, it is about the previous flush call.

> That is, if a flush doesn't complete for any reason, you set
> partial flush to true, and the next time flush is called, you send the
> pending bytes (in fact, the entire segment that contains the first
> byte in the buffer), am I right?
> Maybe I can't see how useful this is 'cause I have not read all the
> code, but if flush simply try to send the entire buffer, and if it
> cannot, simply erase only the sended part and leave the rest to the
> next call?
>

The partialFlush stuff is there to detect if there's a
need to do some extra work on the first segment.  This
function is used with a marshalling library... we only
add to the end of the container.  So the only way the
first segment isn't full is if it also the last segment.



>
>
> >   {
> >     int32_t const chunks = (all - numWritten) / chunk_size;
> >     for (int32_t ii = 0; ii < chunks; ++ii) {
> >       uint32_t bytes = Write(sock_, &buf_[numWritten], chunk_size);
> >       numWritten += bytes;
> >       if (bytes < chunk_size) goto ending;
> >     }
> >   }
>
> Maybe the outer enclosing clock could be eliminated, it's not useful
> here.
>
>
>
> >   if (numWritten < all) {
> >     numWritten += Write(sock_, &buf_[numWritten], all - numWritten);
> >   }
>
> > ending:
> >   buf_.erase(buf_.begin(), buf_.begin() + numWritten);
> >   if (numWritten == all) return true;
>
> >   partialFlush_ = true;
> >   return false;
>
> > }
>
> goto is considered worst practice :) maybe a redesign to eliminate it
> could be better.
>

Thanks for your comments.  I'm not sure about a redesign.
That's why I asked.

[toc] | [prev] | [next] | [standalone]

#10200

> Thanks for your comments.  I'm not sure about a redesign.
> That's why I asked.

mmm maybe I'm missing something, but why you are treating as error a write that send less bytes than what you asked? Why don't you use your PersistentWrite to make sure to send the entire buffer? In general a flush on a stream has the responsibility to empty the buffer sending the entire content to the socket, so its postcondition should be:

assert( buf_.empty() == true );

Why you allow partial flushing, exposing logic to clients of SendBuffer class? For example, the sync() method on standard streambuf have this semantic.

Obviously I don't know much about the context, so partial flush could be a desired behaviour.

Hope this help!

Cheers,
Fulvio

[toc] | [prev] | [next] | [standalone]

#10287

On Sep 15, 8:01 am, Ebenezer <woodbria...@gmail.com> wrote:
> Shalom
>
> How to improve this function?
>
> bool
> SendBuffer::Flush ()
> {
>   uint32_t const all = buf_.size();
>   uint32_t numWritten = 0;
>   if (partialFlush_) {
>     ::neolib::segmented_array<unsigned char, chunk_size>::segment&
> segment =
>                segmented_iterator<unsigned char,
> chunk_size>(buf_.begin()).segment();
>
>     numWritten = Write(sock_, &buf_[0], segment.size());
>     if (numWritten < segment.size()) goto ending;
>     partialFlush_ = false;
>   }

Someone suggested that "goto" isn't optimum solution, so you could
restructure the control flow:

-->

    if (numWritten >= segment.size())
    {
      partialFlush_ = false;
    }
  }

  if (!partialFlush)
  {
    ...
  }

//ending:

The first goto would leave the "partialFlush" into state "true", so
this control flow change would take care of that. The second one is a
break from the for-loop which is easy to reproduce with a signal
variable.

What's the problem you are having with this code? You feel that it
should be more satisfying to the craft or something more functional
than that?

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | comp.lang.c++

csiph-web