Passing const as this discards qualifiers issue

1,991 views
Skip to first unread message

Michael DeForge

unread,
Mar 5, 2016, 1:00:31 PM3/5/16
to actor-framework
I understand that methods declared as const can only call other const methods, however I'm not sure why anything is const here. I'm guessing sync_send takes the second parameter (transform) as a const and therefore returns it as const?

I'm also wondering if there is a better way to do what I'm trying to do... This is only my second day working with CAF and I'm coming from very little Akka experience so be nice :-)

I have a pool of Transforms that I want to consistently apply a downward force of gravity to. Other operations will be taking place on the Transforms, but gravity will always be applied. Here I've created a gravitySystem actor. Right now I don't have multiple Transforms, I just want to apply the gravity transform 10 times. So at the end of the tenth loop, transform.y should be -98.

Obviously, in order to do this, I have to have a running total so that's how I've come to this design...

Compiler Error

g++ -std=c++11 -finstrument-functions -fpermissive -g -I\usr\local\include -o transform transform.cpp -L\usr\local\lib -lcaf_io -lcaf_core
transform.cpp: In lambda function:
transform.cpp:37:19: warning: passing ‘const Transform’ as ‘this’ argument of ‘Transform& Transform::operator=(const Transform&)’ discards qualifiers [-fpermissive]
         transform = t;
                   ^

Code

#include <string>
#include <iostream>


#include "caf/all.hpp"


using std::endl;
using std::string;


using namespace caf;


struct Transform
{
  float x, y, z;
};


behavior gravity(event_based_actor* self) {
  // return the (initial) actor behavior
  return {
    [=](Transform transform) -> Transform {
      transform.y -= 9.8; // m/s
      self->quit();
      return transform;
    }
  };
}


void gravitySystem(event_based_actor* self) {
  Transform transform;
  transform.x = 0.0f;
  transform.y = 0.0f;
  transform.z = 0.0f;
  for (int i = 0; i < 10; i++) {
    auto gravity_actor = spawn(gravity);
    self->sync_send(gravity_actor, transform).then(
      [=](Transform t) {
        aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
        transform = t;
      }
    );
  }
}


int main() {
  spawn(gravitySystem);
  await_all_actors_done();


  shutdown();
}

Lingxi Li

unread,
Mar 5, 2016, 10:07:04 PM3/5/16
to actor-framework
I guess


[=](Transform t) {
  aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
  transform = t;
}

should be

[&](Transform t) {

  aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
  transform = t;
}

By using `[=]`, you are capturing `transform` by value, and since the lambda is not `mutable` specified, the capture value is `const` by default. In other words, the `transform` within the lambda is a const member variable of the lambda object.

After changing `[=]` to `[&]`, you will possibly get rid of the compiler warning, but the program is still flawed. The `sync_send()` thing isn't synchronous (blocking) at all. It returns immediately before the response message is received and the `then` clause is executed. Consequently, you have a data race condition on `transform`. `sync_send()` has been renamed to `request()` in the `topic/actor-system` branch of CAF. The `master` or `develop` branch is pretty much out of date.

Dominik Charousset

unread,
Mar 6, 2016, 5:32:14 AM3/6/16
to actor-f...@googlegroups.com
I guess

[=](Transform t) {
  aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
  transform = t;
}

should be

[&](Transform t) {
  aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
  transform = t;
}

After changing `[=]` to `[&]` (...) the program is still flawed.

Just one addition to Li's answer: that's actually not "flawed", it's straight up undefined behavior.


From the little snippet, it makes little sense to set a variable that is never read. So I assume this is a boiled down version of something bigger? The main issue you have here is to manage state in an event-based actor. Due to the event-based and asynchronous nature of CAF, putting variables on the stack doesn't help you much, as they run out of scope before you receive any message.

Since state management is a common problem, CAF offers state-based actors:

struct gsys_state {
  Transform transform;
  gsys_state() : transform(0.0f, 0.0f, 0.f) {
    // nop
  }
};

void gravitySystem(stateful_actor<gsys_state>* self) {
  for (int i = 0; i < 10; i++) { 
    auto gravity_actor = spawn(gravity); 
    self->sync_send(gravity_actor, transform).then(
      [=](Transform t) {
        aout(self) << "X: " << t.x << " Y: " << t.y << " Z: " << t.z << endl;
        self->state.transform = t;
      }
    );
  }
}



Another solution is to carry the state around in function arguments (which usually has more overhead than the state-based actors).

Hope that helps.

    Dominik

Michael DeForge

unread,
Mar 7, 2016, 8:53:24 PM3/7/16
to actor-framework
Doesn't look like stateful_actors contain a request method, bummer. Now I'm having a problem that might be attributed to that but I'm not sure if that's it and if it is that what to do about it. See "transform.out" and "transform.cpp". Somehow my FOR loop is sending all 10 actors to the behaviors twice (total of 20) before the first for loop completes. Everything is out of order, which is probably why the same Transform is -9.8 twice in a row instead of -9.8 then -19.6 the second time.

And yes, this is a part of something larger. Trying to get the basics down before integrating it. 
transform.out
transform.cpp

Dominik Charousset

unread,
Mar 8, 2016, 5:49:51 AM3/8/16
to actor-f...@googlegroups.com
Doesn't look like stateful_actors contain a request method, bummer.

It does, in the 0.15 topic branch. Sorry for the confusion. sync_send() is going to be renamed to request(). The motivation for renaming this member function is that many users expect sync_send to have blocking semantics (which probably was your assumption as well).


Now I'm having a problem that might be attributed to that but I'm not sure if that's it and if it is that what to do about it. See "transform.out" and "transform.cpp". Somehow my FOR loop is sending all 10 actors to the behaviors twice (total of 20) before the first for loop completes. Everything is out of order, which is probably why the same Transform is -9.8 twice in a row instead of -9.8 then -19.6 the second time.

The for loop does not wait for intermediate results. Event-based actors work asynchronous and callback-based. `.then()` installs a one-shot message handler (callback) to handle the result message. So your loop sends 20 messages, installs 20 response handlers and leaves the function gravitySystem(). Then, the actor receives the response messages and invokes its callback one-by-one.

One way to approach this is to nest sync_send calls:

void gravitySystem(stateful_actor<gsys_state>* self) {
  for (int i = 0; i < 10; i++) {
    auto worker = spawn(gravity);
    aout(self) << "Sending: " << self->state.transform[i].y << std::endl;
    self->sync_send(worker, self->state.transform[i]).then (
      [=](const Transform& t) {
        aout(self) << "Received: " << t.y << std::endl;
        self->state.transform[i] = t;
        aout(self) << "X: " << self->state.transform[i].x << " Y: " << self->state.transform[i].y << " Z: " << self->state.transform[i].z << endl;
        auto worker2 = spawn(gravity);
        self->sync_send(worker2, self->state.transform[i]).then(
          [=](const Transform& t) {
            // ...
          }
        );
      }
    );
  }
}

Your other option is to use a blocking actor instead:

void gravitySystem(blocking_actor* self) {
  actor gravity_actor[10];
  Transform transform[10];
  for (int x = 0; x < 2; x++) {
    for (int i = 0; i < 10; i++) {
      gravity_actor[i] = spawn(gravity);
      aout(self) << "Sending: " << transform[i].y << std::endl;
      self->sync_send(gravity_actor[i], transform[i]).await (
        [&](Transform t) {
          aout(self) << "Received: " << t.y << std::endl;
          transform[i] = t;
          aout(self) << "X: " << transform[i].x << " Y: " << transform[i].y << " Z: " << transform[i].z << endl;
        }
      );
    }
  }
}

You need to spawn your blocking actor with `spawn<blocking_api>(gravitySystem)`. This puts the gravity system into its own std::thread. Each sync_send().await() now blocks until the response arrived (which is why you can now capture by reference in the lambda instead). Since blocking actors require their own thread, they are quite heavyweight. So, usually you want to go for event-based actors if you need performance. Spawning a few blocking actors that model the main application logic shouldn't impact performance, though.

    Dominik



--
You received this message because you are subscribed to the Google Groups "actor-framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to actor-framewo...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
<transform.out><transform.cpp>

Michael DeForge

unread,
Mar 8, 2016, 12:14:07 PM3/8/16
to actor-framework
Wow, so the FOR loop is the fastest thing here. It's able to fire off all 20 "sends" before the function is initialized to even get to "inside". Interesting.

I don't really like either of those options. Got anything more? :-) Traditionally, I'd have an array of Transforms that I'd loop through in a serial manner to update their positions. It's great for cache coherency, but still poor concurrency which is why I'm looking at CAF in the first place. If I have a 100 transforms that need to be updated via a behavior every loop, I want to be able send them out (threading for now, remote actors later), update the state, then repeat. I can't have it repeat before the states are updated from the first loop.

The second method you described sounds kind of close, but I think that essentially causes serial execution where a worker has to wait until the current Transform is finished updating before starting on the next one. Unless I'm wrong?

Thank you for all the help, by the way, this has been immensely helpful and informative. 

Dominik Charousset

unread,
Mar 9, 2016, 9:04:20 AM3/9/16
to actor-f...@googlegroups.com
The second method you described sounds kind of close, but I think that essentially causes serial execution where a worker has to wait until the current Transform is finished updating before starting on the next one. Unless I'm wrong?

You could of course just not use sync_send. Send X transformations out, then wait for X results. This requires your workers to send you their "ID" (position in the array) back:

void gravitySystem(blocking_actor* self) {
  actor gravity_actor[10];
  Transform transform[10];
  for (int x = 0; x < 2; x++) {
    for (int i = 0; i < 10; i++) {
      gravity_actor[i] = spawn(gravity);
      aout(self) << "Sending: " << transform[i].y << std::endl;
      self->send(gravity_actor[i], transform[i], i);
    }
    int i = 0;
    self->receive_for(i, 10)(
      [&](Transform t, int id) {
        aout(self) << "Received: " << t.y << std::endl;
        transform[id] = t;
        aout(self) << "X: " << transform[id].x << " Y: " 
                   << transform[id].y << " Z: " << transform[id].z << endl;
      }
    );
  }
}

Thank you for all the help, by the way, this has been immensely helpful and informative. 

You're very welcome. This is what the mailing list is there for. :)

    Dominik

Michael DeForge

unread,
Mar 11, 2016, 4:51:45 PM3/11/16
to actor-framework
Hi. To get both Transform and ID back from my worker, do I need to use a tuple? Is that implied here or is [&](Transform t, int id) really possible?

Dominik Charousset

unread,
Mar 14, 2016, 6:03:12 AM3/14/16
to actor-f...@googlegroups.com
Hi. To get both Transform and ID back from my worker, do I need to use a tuple? Is that implied here or is [&](Transform t, int id) really possible?

You need to send the ID manually:

behavior gravity(event_based_actor* self) {
  // return the (initial) actor behavior
  return {

    [=](Transform transform, int transform_id) -> std::tuple<Transform, int> {

      transform.y -= 9.8; // m/s
      self->quit();

      return std::make_tuple(std::move(transform), transform_id);
    }
  };
}

Note that the tuple is "unboxed" by CAF automatically, so your response handler is simply "[&](Transform t, int id)".

    Dominik

Reply all
Reply to author
Forward
0 new messages