Re: [boost] [context review] Review starts today, Mars 21st 2011

Subject: Re: [boost] [context review] Review starts today, Mars 21st 2011
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2011-03-30 03:33:09

On Wed, Mar 30, 2011 at 12:04 AM, Oliver Kowalke <oliver.kowalke_at_[hidden]>wrote:

> Hi Jeffrey,
>
> > Oliver has put in the time for this library, along with Boost.Fiber and
> > Boost.Coroutine.
>
> boost.coroutine was written by Giovanni Piero Deretta!
> I'm responsible for boost.fiber/boost.tasklet/boost.task - whic hare
> removed from the boost vault until boost.context isready.
>

My mistake!

> At this point, given that I'm not at all familiar with context technology
> > and terminology, the "user-level" in "user-level context" seems
> > superfluous.  I can only guess that there are "kernel-level" contexts...
>
> yes - the kernel schedules kernel-level contexts
>

I'd only suggest an additional footnote mentioning kernel-level contexts for
the unfamiliar such as myself, although if you don't think it's necessary,
then don't bother.

> *** How to build and install ***
> > So, first you claim "Boost.Context provides an abstraction
> > (boost::contexts::native_impl)...", then go on to say that
> > "...boost::contexts::native_impl might not be available".
>
> modification in the lib: fcontext_t will be used in favour of ucontext_t if
> available
>

Great.

> Yikes, I'm scared by all these build parameters.  Any way we can make
> > building one implementation or the other easier?  Or maybe expand this
> > section on how to build the fcontext implementation?
>
> this is due a limitation of boost.build/bjam - Vladimir works on a new
> version of boost.build which provides architecture, instructionset,
> address-model as general properties instead of optional properties.
>

Sounds good.

> It seems the only (documented) drawback with using asm_impl is failure to
> > preserve signal masks and inability to be called by asynchronous signal
> > handlers.  Are these sufficiently useful to preclude hiding which context
> > implementation is used?  Any code that requires signal mask preservation
> > and/or asynchronous signal handlers would have to conditionally use those
> > anyway to remain cross-platform, right?
>
> ucontext_t preserves signal masks so that the code in a ucontext_t can
> install it own signal mask.
> boost.context requires that the code exyecuted by boost.context will not to
> change the signal mask.
>

That wasn't documented anywhere that I could see, so I presume this is a
recent requirement?

> > I would guess, but I'd have to think hard about it, that any program
> which
> > uses boost::context must create at least two boost::context objects, and
> > at
> > least one of these objects must reference the current context.  Is this
> > the
> > case?
>
> two context objects are required because you want to jump from one context
> to another (and maybe back)
>

Okay.

> > To switch back to the stalled context before the switched-to
> > context's function returns, the stalled context must be passed (through
> > the
> > void* argument) to the switched-to context's function, right?
>
> not sure what you are refering to - the void pointer in make_fcontext() is
> used to transport user data to the called fucntion
>

This was more of an observation than a question.

The user data could include a reference to the stalled context, which would
effectively give you a rudimentary coroutine: you would be able to return
control back to the stalled context at any point during the execution of the
switched-to context.

> > Indeed, it's not clear *where* a protected_stack object allocates
> > its memory from; at the very least *this* should be documented.
> > - I don't know that I agree that a stack must protect against exceeding
> > the
> > stacksize (although protected_stack certainly must!)...surely there are
> > situations where you *know* you have enough stack space for your
> > particular
> > application, and hence guard pages would be unnecessary.
>
> the used stack should always! protected (==using a guard page at the end)!
> Imagine you implement a recursive descent parser (recursive invokcation of
> a parsing function). it depends from the input how much recursive
> invocations (with local data) your parse as to call in order to parse your
> text. That means every call to the parsing function consumes stackspace.
> If you reach the end of your stack two thins may happen if you do
> additional recursive calls to the parsing function.
>
> 1.) you access memory not belonging to your process -> segmentation
> fault/access violation
>
> 2.) or you overwrite your own memory which will result in
> unexpected/undefined behaviour of your app
>

Yes, I understand; this is why I used the phrase "there are situations
where" rather than, e.g., "all situations are such that".

Please note, a guard page doesn't allocate space from the physical memory
> (its a special entry in the page table).

Yes, but requiring a guard page does limit where you can put your stack's
memory.  E.g., should I be able to do

    char stack_space[256]; // okay, not properly aligned; use
boost::type_with_alignment to get the right alignment
    context c(..., stack_space);

as long as I *know* that the function which context c references will not
use more than 256 bytes of stack space?  Let's just suppose that on all
platforms I plan to support with the above code, 256 bytes is enough.  Is
this just a bad idea?  If so, I think it warrants discussion in the
documentation, and if not, I think the guard page requirement should be
changed to a *recommendation*.

>  It doesn't look
> > like there's a way to determine the amount of free space / used space in
> a
> > stack...is such a thing possible and/or useful?
>
> boost::context::detail:.stack_helper provides three static functions
>
> maximal_stacksize()
> minimal_stacksize()
> default_stacksize()
>

The fact that they are in a detail namespace renders them essentially
invisible from a user's perspective.  So, again, are these functions
useful?  Should they be exposed in the public interface?  Should they be
requirements of the stack concept?

> - operator [unspecified-bool-type() and operator!() were confusing to me
> > at
> > first, as it left me wondering how a protected_stack object could be
> > anything other than a stack!  I think, if default-constructible stacks
> > (i.e., not-a-stack stacks) are deemed useful or necessary, they should be
> > called something other than "not-a-stack", and more descriptively named
> > member functions should be used to query the non-a-stack state.
>
> move semantics of stack require to express a 'not-a-stack' which means the
> stack content was moved. please look in the docu of boost.thread where
> Anthony explains it better than me.
>

Right, of course, that makes sense.  Please add this observation to the
documentation.

> For the assembler implementation of context switching, is it possible to
> > explicitly calculate the number of cycles, via a summing of the cycle
> > counts
> > of the instructions?
>
> at least for x86 boost.context provides a tool counting the cycles for a
> switch (libs/context/performance)
>

I'm sorry to be picky, but I tend to think that the methodology one uses to
generate performance statistics should be spelled out very explicitly.  I
assume you used this tool to generate all the cycle counts in the table.
I'm guessing it's not a static analysis tool (i.e., it doesn't actually run
the code, which is what I was referring to in my comment), but rather a
runtime analysis tool, judging by your performance notes, correct?

 - Jeff