|
Boost : |
Subject: Re: [boost] [context review] Performance questions
From: Antony Polukhin (antoshkka_at_[hidden])
Date: 2012-01-10 13:32:19
Review
Here are some small remarks:
1) Section "Struct boost_fcontext_t and related functions" misses
header name, in which C interface is described.
2) Header context/detail/context_base.hpp includes
# include <boost/context/detail/context_base_fiber.hpp>
# include <boost/context/detail/context_base_uctx.hpp>
This headers do not exist in checkout.
3) Moving context between threads is described in documentation but is
not tested in tests.
4) Rvalue references sometimes miss forward(). For example
bost/context/context.hpp (line 60) must be like:
template< typename Fn, typename Allocator >
static base_ptr_t make_context_(
Fn && fn, std::size_t size, flag_unwind_t do_unwind,
flag_return_t do_return, Allocator const& alloc)
{
return base_ptr_t(
new detail::context_object< typename remove_reference< Fn
>::type, Allocator >(
boost::forward<Fn>(fn), alloc, size, do_unwind, do_return) );
}
Also forward() calls are required on lines 76, 151, 156, 161 (instead
of static_cast< &&>)
Preprocessor generated constructors can also have their parameters forwarded.
Test cases can be add for this (create function object that is only
movable, and move it to the context instance)
I`ve spend 2-3 hours looking through the code. Code looks good. Did
not build or run tests or examples. Acceptance conditions have been
met.
I vote "yes".
One more question: does ARM implementation works correctly
with/without VFP registers/floating point support?
Best regards,
Antony Polukhin
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk