views:

106

answers:

3

I'm trying to achieve the following optimization in my container library:

  • when inserting an lvalue-referenced element, copy it to internal storage;
  • but when inserting rvalue-referenced element, move it if supported.

The optimization is supposed to be useful e.g. if contained element type is something like std::vector, where moving if possible would give substantial speedup.

However, so far I was unable to devise any working scheme for this. My container is quite complicated, so I can't just duplicate insert() code several times: it is large. I want to keep all "real" code in some inner helper, say do_insert() (may be templated) and various insert()-like functions would just call that with different arguments.

My best bet code for this (a prototype, of course, without doing anything real):

#include <iostream>
#include <utility>

struct element
{
  element () { };
  element (element&&) { std::cerr << "moving\n"; }
};

struct container
{
  void  insert (const element& value)
  {  do_insert (value);  }

  void  insert (element&& value)
  {  do_insert (std::move (value));  }

private:
  template <typename Arg>
  void  do_insert (Arg arg)
  {  element  x (arg);  }
};

int
main ()
{
  {
    // Shouldn't move.
    container  c;
    element x;
    c.insert (x);
  }

  {
    // Should move.
    container  c;
    c.insert (element ());
  }
}

However, this doesn't work at least with GCC 4.4 and 4.5: it never prints "moving" on stderr. Or is what I want impossible to achieve and that's why emplace()-like functions exist in the first place?

+2  A: 

I think you may need to forward the argument:

  template <typename Arg>
  void  do_insert (Arg&& arg)
  {  element  x (std::forward<Arg>(arg));  }

Full code:

#include <iostream>
#include <utility>

struct element
{
  element () { };
  element (const element&) { std::cerr << "copying\n"; }
  element (element&&) { std::cerr << "moving\n"; }
};

struct container
{
  void  insert (const element& value)
  {  do_insert (value);  }

  void  insert (element&& value)
  {  do_insert (std::move(value));  }

private:
  template <typename Arg>
  void  do_insert (Arg&& arg)
  {  element  x (std::forward<Arg>(arg));  }
};

int
main ()
{
  {
    // Shouldn't move.
    container  c;
    element x;
    c.insert (x);
  }
  {
    // Should move.
    container  c;
    c.insert (element ());
  }
}

The keyword that you might look for is "perfect forwarding".

UncleBens
If I do that, I get two "moving"s on output. But I guess I need to read some documentation on that function.
doublep
After edit the code works and lacks the weird explicit <> arguments to `do_insert()`, unlike my self-answer. Thanks!
doublep
A: 

I'd recommend to copy the way it's done in your STL implementation (or GNU, which should be able to read online anyway).

But

  template <typename Arg>
  void  do_insert (Arg arg)
  {  element  x (move(arg));  }

might do the trick.

This functionality is separate from emplace and you're correct that it works in the standard library.

EDIT I made some changes and discovered

  • When you get two messages, there is one from each insertion
  • The second insertion appears to be fine
  • The first generates a spurious move but the original object was not moved. So focus on finding a temporary which is being moved… this isn't actually such a bad thing.

.

#include <iostream>
#include <utility>

struct element
{
  element () : moved(false) { };
  element (element&&) { moved = true; std::cerr << "moving\n"; }
  bool moved;
};

struct container
{
  void  insert (const element& value)
  {  do_insert (value);  }

  void  insert (element&& value)
  {  do_insert (std::move (value));  }

private:
  template <typename Arg>
  void  do_insert (Arg arg)
  {  element  x (std::move(arg));  }
};

int
main ()
{
  std::cerr << "try 1\n";
  {
    // Shouldn't move.
    container  c;
    element x;
    c.insert (x);
    std::cerr << x.moved << "\n";
  }

  std::cerr << "try 2\n";
  {
    // Should move.
    container  c;
    c.insert (element ());
  }
}
Potatoswatter
It is not implemented for similar containerd (`unordered_*`) in GNU STL. That might indicate that it is impossible to do (easily) or maybe it's just not implemented yet. About forwarding not moving: maybe, but it doesn't help either.
doublep
@doublep: My point is actually that you need to modify `do_insert`. GNU (and presumably everyone else) handles this step with the allocator, so it shouldn't matter which container. I'm not 100% sure whether to use move or forward, but the lack of either in your `do_insert` is clearly a bug.
Potatoswatter
@Potatoswatter: No matter what I add in `do_insert()`, I get two "moving" messages. I.e. it might well be a bug, but I have no idea how to solve it.
doublep
@doublep: see changes.
Potatoswatter
When I run this updated code, I get "moving" message in both cases. Also, I think `moved` flag is wrong, because what you do in printing `x.moved` is (logically) checking for "`x` was moved away", not "`x` was created with a move constructor".
doublep
@doublep: The only thing we're creating with a move constructor is the local. For the diagnostic I printed, that is what I was looking for. We want to make sure that `x` was not moved away.
Potatoswatter
@Potatoswatter: then assignment to `moved` should look like `that.moved = true` where `that` is the move constructor argument, otherwise your output line doesn't match your intentions. I posted a self-answer that *seems* to do what I want and builds on your hints.
doublep
@doublep: aha, lol in my other program to trace the behavior of my STL implementation I did it the right way. Cool that you got it working…
Potatoswatter
A: 

I can't say I understand why this works and some other code doesn't, but this seems to do the trick (created thanks to hints from Potatoswatter):

#include <iostream>
#include <utility>

struct element
{
  element () { };
  element (const element&) { std::cerr << "copying\n"; }
  element (element&&) { std::cerr << "moving\n"; }
};

struct container
{
  void  insert (const element& value)
  {  do_insert <const element&> (value);  }

  void  insert (element&& value)
  {  do_insert <element&&> (std::forward <element&&> (value));  }

private:
  template <typename Arg>
  void  do_insert (Arg arg)
  {  element  x (std::forward <Arg> (arg));  }
};

int
main ()
{
  std::cerr << "1\n";
  {
    // Shouldn't move.
    container  c;
    element x;
    c.insert (x);
  }

  std::cerr << "2\n";
  {
    // Should move.
    container  c;
    c.insert (element ());
  }
}

I get the following output with both GCC 4.4 and 4.5:

1
copying
2
moving
doublep