1

For timeouts in event reactors and proactors I use a priority queue that also allows O(log(n)) random access removes of events (when the event is signalled/completes rather than a timeout occurring). I store std::pair<std::chrono::steady_clock::time_point, Timed *> where Timed is a class that adds has an index (pointing into the queue) to allow efficient removal when calling TimedQ::Remove(Timed *p). When I want to have an event type associated with a timeout, I derive from Timed. The queue's Top() and Pop() return a pair.

I used to have a a bunch of code using the queue such as

std::tie(timePt0, eventPtr0) = timeoutQ.Pop();
std::tie(timePt1, eventPtr1) = std::move(hold);

which worked fine before I started using a base class Timed * in the queue instead of specific event type (i.e. Timed was originally a templated type instead), as I eventually needed to support multiple different event types that can be associated with the timeouts. However, with eventPtr* being a derived type (that I can static_cast to from a Timed * returned by the queue), code like the above no longer works.

I'm wondering what's the best way to do this. Right now, it's ended up very verbose, and I'm concerned about efficiencies like temporaries being created as well:

auto v(timeoutQ.Pop());
timePt0 = v.first;
eventPtr0 = static_cast<TimedEvent *>(v.second);
std::tie(timePt1, eventPtr1) = std::move(std::make_pair(hold.first, static_cast<TimedEvent *>(hold.second)); // I didn't literally do it like this, but I'm just trying to illustrate my struggle

The only other idea I had was to template the functions that return a pair by the derived event class, but this seems poor from a code size perspective, as multiple instances of those functions will be created even though the machine code should be identical since in all cases it's a pointer that's stored.


Edit: I also tried using this, which compiles, but I'm not sure is correct or efficient:
template<class D>
std::pair<std::chrono::steady_clock::time_point, D *> &&Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed *> &&in)
{
    return std::make_pair(in.first, static_cast<D *>(in.second));
}

the initial example then would become

std::tie(timePt0, eventPtr0) = Cnvrt<std::remove_pointer<decltype(eventPtr0)>::type>(timeoutQ.Pop());
std::tie(timePt1, eventPtr1) = Cnvrt<std::remove_pointer<decltype(eventPtr1)>::type>(hold);
Display Name
  • 2,323
  • 1
  • 26
  • 45
  • 3
    the verbosity is a real problem. But efficiency is not. Compiler optimizations are very advanced, especially for local optimizations like in this example. If you don't trust me you can look at the assembler and/or profile. – bolov Jan 28 '16 at 07:16
  • I guess you don't need the `std::move`, since make_pair is returning an rvalue BTW. – Mehrdad Jan 28 '16 at 07:32
  • `static_cast` has almost no run-time overhead. so you can't have efficiency issue because of that. – Mehrdad Jan 28 '16 at 07:37
  • I don't have a problem with the cast -- I'm fine to specifically declare to what I'm downcasting -- but with temporaries (including manual ones like the variable v in the example), and also the verbosity. – Display Name Jan 28 '16 at 07:40
  • No one can tell you better than the compiler and profiler if the code is faster or not. – Mehrdad Jan 28 '16 at 08:25

1 Answers1

1

The Cnvrt you've shown returns a dangling reference – classic UB.

Here's a corrected C++11-compliant version that also validates D at compile-time and removes the need for the manual std::remove_pointer<...>::type at the call site:

template<typename D>
constexpr
std::pair<std::chrono::steady_clock::time_point, D>
Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed*> const& in) noexcept
{
  static_assert(std::is_pointer<D>{}, "D is not a pointer type");

  using derived_type = typename std::remove_pointer<D>::type;
  static_assert(std::is_base_of<Timed, derived_type>{}, "D does not derive from Timed");

  using ptr_type = typename std::remove_cv<D>::type;
  return {in.first, static_cast<ptr_type>(in.second)};
}

// ...

std::tie(timePt0, eventPtr0) = Cnvrt<decltype(eventPtr0)>(timeoutQ.Pop());
std::tie(timePt1, eventPtr1) = Cnvrt<decltype(eventPtr1)>(hold);

Online Demo

Here is an implementation that should work on VC++ 2012:

template<typename D>
std::pair<std::chrono::steady_clock::time_point, D>
Cnvrt(std::pair<std::chrono::steady_clock::time_point, Timed*> const& in) throw()
{
  static_assert(std::is_pointer<D>::value, "D is not a pointer type");

  typedef typename std::remove_pointer<D>::type derived_type;
  static_assert(std::is_base_of<Timed, derived_type>::value, "D does not derive from Timed");

  typedef typename std::remove_cv<D>::type ptr_type;
  return std::make_pair(in.first, static_cast<ptr_type>(in.second));
}

Online Demo

There is no efficiency concern here whatsoever – even your worst-case scenario, if the compiler does no optimizations at all, is just a copy of one scalar and one pointer (VC++ 2012 may copy each twice, but again, only without optimizations enabled).

ildjarn
  • 62,044
  • 9
  • 127
  • 211
  • Thanks for this solution. A couple of questions: What's the penalty for removing `constexpr`? I'm on VC++2012 and it's not supported. It also support neither `using` for templated typedefs nor `return {}`. I rewrote the body as follows: `static_assert(std::is_pointer::value, "D is not a pointer type"); static_assert(std::is_base_of::type>::value, "D is not derived from Timed"); return std::pair::type>(in.first, static_cast::type>(in.second));` Does this look OK? It compiles at least... – Display Name Jan 29 '16 at 05:40
  • In the above, I noticed that `is_pointer` already does `remove_cv` at least in this compiler; `remove_pointer` by removing the pointer also removes its qualifiers; and `is_base_of` doesn't seem to be affected by qualifiers when I tested anyway. Seeing that the code removes const/volatile qualifiers from `D`, is there any way to, instead, template `Timed` and then transfer its qualifiers to `D`? – Display Name Jan 29 '16 at 05:43
  • @DisplayName : Answer updated. You're right that there were some redundancies. `constexpr` doesn't matter if you're compiler can't use it. Also, in the future, if you're asking about a deficient compiler such as VC++ 2012, please make sure to mention it in the question. :-] – ildjarn Jan 29 '16 at 14:03
  • @DisplayName : "*Seeing that the code removes const/volatile qualifiers from `D`, is there any way to, instead, template `Timed` and then transfer its qualifiers to `D`?*" With the most recent edit, passing `TimedEvent volatile* const` for `D` returns `pair`. Is this not what's desired? If not, then given that `D`, what return type _is_ expected? `pair` or `pair`? – ildjarn Jan 29 '16 at 14:33
  • Thanks for the update. I assume in this specific use case, `return std::make_pair()` is just less verbose than `return std::pair<>()` and has no other difference? – Display Name Jan 29 '16 at 20:44
  • 1
    @DisplayName : Right, that and VC++ 2012's `pair` has some strange non-standard constructors (meant as a first stab at perfect forwarding) that can fail when explicitly specifying types and passing in different (but convertible) types. This was cleaned up in VC++ 2013 IIRC. – ildjarn Jan 29 '16 at 20:46