do not call actor for inspect and hash identity #161

Closed
wants to merge 2 commits into
from

Projects

None yet

2 participants

@pberndt

Hello Tony,

thanks a lot for celluloid and celluloid-io!

I am currently using it for developing a benchmarking framework. For this I needed to make a couple of changes. While debugging my application I ran into problems where inspecting the actor proxies created infinite loops until I commented out the call to the actor in ActorProxy#inspect and only show information local to the proxy. At this, I also found it insightful while debugging to see my object is actually an ActorProxy, not an actor.

In a similar vein, I found it practical to be able to put actor proxies into a Hash. For this to work in the case where actors may die, the identity information should not be queried from the actors. Using local information is also much faster, especially when the actors are busy. This also enables me to see in trap_exit which particular actor died more than just its class.

Best regards,
Philipp

Philipp Berndt added some commits Jan 30, 2013
Philipp Berndt have ActorProxy::inspect inspect the proxy, not the actor
More robust, e.g. does not lead to infinite recursion and
deadlocks.
fe9d23b
Philipp Berndt provide hash identity for actor proxies without calling the actor.
Results in better performance and also works with proxies whose agents
may have died.
Also allows you to recognize a specific agent in trap_exit callback.
32fd19e
@tarcieri
Celluloid member

Well, there's some stuff I like here and some stuff I don't.

Changing "Celluloid::Actor" to "Celluloid::ActorProxy" in #inspect makes a lot of sense.

That said, I do like rich inspect for actors and I imagine others do too. Regarding the infinite loop problem, you might do some reading on the issue here: #22

I do intend to have a recursion detection system in place soon, which will mitigate the issue you're trying to resolve here.

Furthermore, #hash is already defined on the proxy object and the actor is not consulted for this (just double checked that). What led you to believe that #hash was consulting the actor?

@tarcieri
Celluloid member

a4fb4b1 changes Celluloid#inspect to display Celluloid::ActorProxy instead of Celluloid::Actor

@pberndt

That said, I do like rich inspect for actors and I imagine others do too.

I guess it's a matter of taste. Personally I use inspect only for debugging. However, an actor-based inspect call could block and change the call sequence and behavior of the application when enabling debugging output. This could possibly result in heisenbugs that disappear when you try to look at them.
Maybe there should be another way to inspect the proxy only? Could we at least have a second, less invasive method that only prints local information?

Furthermore, #hash is already defined on the proxy object and the actor is not consulted for this (just double checked that). What led you to believe that #hash was consulting the actor?

You are right! Your version actually passes my "can be stored in hashes" rspec example. Don't know what it was that led me to believe otherwise. (Maybe the thought that one actor could have multiple proxies?) Anyway, sorry.

@tarcieri
Celluloid member

I updated inspect to reflect ActorProxy. Calling that good enough?

@tarcieri tarcieri closed this Feb 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment