0

I am working with a C++ library that was not written by me. Currently, I am trying to improve the library a bit by removing a lot of circular dependencies.

The libray communicates over the network and has some message classes, which are created when reading data received from the network.

Currently, it works like this:

  • The message is parsed and a std::unique_ptr<Message> is being created.
  • The Message is passed to the parent object with std::move(msg)

I removed the circular dependency from the network parser to the parent object and used a signal instead, which emits a std::shared_ptr<Message>. I am wondering if it would be a bad idea to just pass the Message by value instead of a shared_ptr. Would this decrease the performance?

The library passes many objects with unique_ptr like in this case. Is this good practice?

Thanks in advance

EDIT:

The Message consists of three unsigned int and a std::map<string, string> which will not hold more than a few strings

mario.schlipf
  • 1,257
  • 2
  • 13
  • 29
  • If you receive the message over the network copying would probably not be a problem. – MB-F Feb 19 '14 at 10:22
  • so pass the object by value instead of the unique_ptr? I think this would be much more readable. Or is there any disadvantage? – mario.schlipf Feb 19 '14 at 10:25
  • Passing by value will certainly copy the map. If that is feasible depends on your application. If you process a message only once in a while and do not care much about delays in passing the message it should be feasible. I'm afraid only profiling can give you a definitive answer, though. – MB-F Feb 19 '14 at 10:29

2 Answers2

0

How big is the message, and are the big parts movable?

If your message is, say, an int and two std::strings, the move constructor should be pretty fast. Whether that degrades your performance depends on how much you do it.

However, beware of copying the message if it's large.

Based on the additional info: pass it by value. If you can, use a single std::function instead of a boost::signal, because that allows you to move the message.

Pass by value is, as you say, preferable due to its readability, and should always be your first choice. Only if that shows to be slow in profiling should you look for other solutions.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
  • I edited the question with your requested info. Would it be better to use unique_ptr or just pass things by value, which, in my opinion, leads to much more readable code. – mario.schlipf Feb 19 '14 at 10:23
  • When using a single std::function, this might not be thread safe, is it? I thought boost::signals2 is thread safe – mario.schlipf Feb 19 '14 at 10:49
  • Registering in signals2 is thread-safe. With std::function, assignment is not. But do you need that? Does your class actually start receiving messages in one thread while the callback isn't definitely registered yet? – Sebastian Redl Feb 19 '14 at 12:04
0

In general, non-const objects wrapped into shared_pointers don't fit well in inherently asynchronous and threaded applications. The application can get much more complex due to preventing potential data races.

It would be probably better to pass objects by value or unique_pointer, unless those objects are const.

Regarding performance: you can utilize move semantics.

CouchDeveloper
  • 18,174
  • 3
  • 45
  • 67
  • what do you mean with utilizing move semantics? By passing by value? – mario.schlipf Feb 19 '14 at 10:40
  • I thought that by using shared_pointers/unique_pointers, the use of the objects are thread safe – mario.schlipf Feb 19 '14 at 10:41
  • @schlimpf Move semantics: see SO: http://stackoverflow.com/questions/3106110/what-is-move-semantics. Regarding thread-safety: shared_pointer is itself thread-safe, the object where it's pointing to (possibly) requires additional synchronization. – CouchDeveloper Feb 19 '14 at 10:42