Refactoring of 0.15 API #468

Closed
Neverlord opened this Issue Jun 2, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@Neverlord
Member

Neverlord commented Jun 2, 2016

This issue gathers refactoring requests for 0.15 and announces upcoming changes. The idea is to give all "early adopters" a platform for discussion and to announce upcoming changes a few days in advance instead of silently breaking the API every now and then.

The first API update will remove the virtual member function actor_system_config::init. As pointed out by @mavam in #467, users can simply perform initialization in the default constructor of subclasses. This change will be merged to develop on June 6.

Also raised by @mavam: actor handles don't use a consistent term for describing "null handles": unsafe_actor_handle_init, x.unsafe(), and invalidate(x). The alternative proposal was to go with "invalid" in all three cases. I originally preferred "unsafe" over "invalid", because it is more descriptive. A handle in this state is unsafe to use, i.e., can cause undefined behavior. Are there more opinions on this? Maybe I could be convinced to go with invalid_handle_init, x.invalid(), and invalidate(x).

@mavam

This comment has been minimized.

Show comment
Hide comment
@mavam

mavam Jun 2, 2016

Member

The alternative proposal was to go with "invalid" in all three cases. I originally preferred "unsafe" over "invalid", because it is more descriptive. A handle in this state is unsafe to use, i.e., can cause undefined behavior. Are there more opinions on this?

Just to clarify my position: using something called "unsafe" in user code gives me pause. It feels like I'm doing something wrong, violating some invariants, or breaking things. This doesn't seem to be the case here. I'm just using the public API as it is available. For these reasons, I prefer more gentle vocabulary.

One step further: Instead of invalid_handle_init, how about invalid_handle? The initialization is implicit in the action of the code, i.e., if one can only use it in the ctor, then it's just a redundant suffix that adds noise.

Member

mavam commented Jun 2, 2016

The alternative proposal was to go with "invalid" in all three cases. I originally preferred "unsafe" over "invalid", because it is more descriptive. A handle in this state is unsafe to use, i.e., can cause undefined behavior. Are there more opinions on this?

Just to clarify my position: using something called "unsafe" in user code gives me pause. It feels like I'm doing something wrong, violating some invariants, or breaking things. This doesn't seem to be the case here. I'm just using the public API as it is available. For these reasons, I prefer more gentle vocabulary.

One step further: Instead of invalid_handle_init, how about invalid_handle? The initialization is implicit in the action of the code, i.e., if one can only use it in the ctor, then it's just a redundant suffix that adds noise.

@Neverlord

This comment has been minimized.

Show comment
Hide comment
@Neverlord

Neverlord Jun 4, 2016

Member

using something called "unsafe" in user code gives me pause. It feels like I'm doing something wrong, violating some invariants, or breaking things.

Giving you pause is exactly what I wanted the name to do. You actually are breaking an invariant here: setting a handle that is supposed to be non-null to null. This is a ticket to good ol' undefined behavior land.

As it turned out in chat, you were trying to use a handle like a pointer (0.14 style). Consequently, the thing you should be reaching for instead is weak_actor_ptr. So, it seems to me that unsafe_ is a good term.

Probably we should instead rename invalidate(x). I was thinking about destroy(x) instead. The use case for this function is to break cycles manually. So it should indicate that using x afterwards is not safe.

I'll add a bit more about weak_actor_ptr to the manual next week, btw.

Member

Neverlord commented Jun 4, 2016

using something called "unsafe" in user code gives me pause. It feels like I'm doing something wrong, violating some invariants, or breaking things.

Giving you pause is exactly what I wanted the name to do. You actually are breaking an invariant here: setting a handle that is supposed to be non-null to null. This is a ticket to good ol' undefined behavior land.

As it turned out in chat, you were trying to use a handle like a pointer (0.14 style). Consequently, the thing you should be reaching for instead is weak_actor_ptr. So, it seems to me that unsafe_ is a good term.

Probably we should instead rename invalidate(x). I was thinking about destroy(x) instead. The use case for this function is to break cycles manually. So it should indicate that using x afterwards is not safe.

I'll add a bit more about weak_actor_ptr to the manual next week, btw.

@mavam

This comment has been minimized.

Show comment
Hide comment
@mavam

mavam Jun 6, 2016

Member

Consequently, the thing you should be reaching for instead is weak_actor_ptr

Yep, I'll adapt my code accordingly. I think the semantics are equivalent to optional<actor_addr>, which was what you proposed in the chat. Except that weak_actor_ptr is the first-class structure that enables the that.

Probably we should instead rename invalidate(x). I was thinking about destroy(x) instead.

Whatever makes semantically more sense. If it's more about destruction that sheer invalidation, then we should go with that.

The use case for this function is to break cycles manually. So it should indicate that using x afterwards is not safe.

Member

mavam commented Jun 6, 2016

Consequently, the thing you should be reaching for instead is weak_actor_ptr

Yep, I'll adapt my code accordingly. I think the semantics are equivalent to optional<actor_addr>, which was what you proposed in the chat. Except that weak_actor_ptr is the first-class structure that enables the that.

Probably we should instead rename invalidate(x). I was thinking about destroy(x) instead.

Whatever makes semantically more sense. If it's more about destruction that sheer invalidation, then we should go with that.

The use case for this function is to break cycles manually. So it should indicate that using x afterwards is not safe.

@Neverlord

This comment has been minimized.

Show comment
Hide comment
@Neverlord

Neverlord Jun 23, 2016

Member

As discussed in #465: all I/O functions in the middleman and in brokers will return expected<T> and no longer throw. For example, remote_actor and publish.

Also, blocking actors "lose" set_error_handler, set_down_handler, and set_exit_handler. Instead, blocking actors will receive all messages via their mailbox and are required to handle exit_msg, down_msg, etc. manually. This primarily affects scoped_actor and ultimately makes it easier to bridge actor and non-actor code. receive and friends will get a way to define catch-all rules.

actor_system will lose its default constructor. This makes actor_system_config mandatory. This removes an ugly hack to get system.config() working if no user-provided config exists and makes sure the config always lives somewhere on the stack as opposed to silently heap-allocated configs.

The changes will go live tomorrow or Monday (with updated manual). I think this set of changes rounds up the API for now and we can ship the first pre-release mid next week.

Member

Neverlord commented Jun 23, 2016

As discussed in #465: all I/O functions in the middleman and in brokers will return expected<T> and no longer throw. For example, remote_actor and publish.

Also, blocking actors "lose" set_error_handler, set_down_handler, and set_exit_handler. Instead, blocking actors will receive all messages via their mailbox and are required to handle exit_msg, down_msg, etc. manually. This primarily affects scoped_actor and ultimately makes it easier to bridge actor and non-actor code. receive and friends will get a way to define catch-all rules.

actor_system will lose its default constructor. This makes actor_system_config mandatory. This removes an ugly hack to get system.config() working if no user-provided config exists and makes sure the config always lives somewhere on the stack as opposed to silently heap-allocated configs.

The changes will go live tomorrow or Monday (with updated manual). I think this set of changes rounds up the API for now and we can ship the first pre-release mid next week.

@Neverlord

This comment has been minimized.

Show comment
Hide comment
@Neverlord

Neverlord Jun 27, 2016

Member

I'm still clearing a few internals up, etc. Update will land hopefully tomorrow.

Member

Neverlord commented Jun 27, 2016

I'm still clearing a few internals up, etc. Update will land hopefully tomorrow.

@Neverlord

This comment has been minimized.

Show comment
Hide comment
@Neverlord

Neverlord Jun 29, 2016

Member

Ok, changes are live in develop.

I think the API changes are a huge step in the right direction. The new abstractions and in particular the configurability sets us on a good path towards 1.0. Special thanks to all early adopters for participating, discussing, and bug fixing!

I will focus on bug fixing from here on out, except #470. If nothing dramatic pops up, we will release the first pre-release version for 0.15 tomorrow (June 30). Feedback in any shape or form is highly welcome. So are bug reports. :)

My hope is to stabilize 0.15 quickly and ship a second pre-release end of July with #470 implemented. Whether or not we will need another pre-release depends on user input and of course how many bugs show up.

@ukreator CAF no longer catches exceptions automatically if you use the configure script option --no-exceptions (or pass set CAF_NO_EXCEPTIONS to yes in CMake). In the long term, CAF will also no longer throw any exceptions except for critical errors. Setting CAF_NO_EXCEPTIONS turns those exceptions into abort(). Currently, CAF can still throw exceptions while serializing/deserializing stuff. This will be resolved eventually with #470.

Member

Neverlord commented Jun 29, 2016

Ok, changes are live in develop.

I think the API changes are a huge step in the right direction. The new abstractions and in particular the configurability sets us on a good path towards 1.0. Special thanks to all early adopters for participating, discussing, and bug fixing!

I will focus on bug fixing from here on out, except #470. If nothing dramatic pops up, we will release the first pre-release version for 0.15 tomorrow (June 30). Feedback in any shape or form is highly welcome. So are bug reports. :)

My hope is to stabilize 0.15 quickly and ship a second pre-release end of July with #470 implemented. Whether or not we will need another pre-release depends on user input and of course how many bugs show up.

@ukreator CAF no longer catches exceptions automatically if you use the configure script option --no-exceptions (or pass set CAF_NO_EXCEPTIONS to yes in CMake). In the long term, CAF will also no longer throw any exceptions except for critical errors. Setting CAF_NO_EXCEPTIONS turns those exceptions into abort(). Currently, CAF can still throw exceptions while serializing/deserializing stuff. This will be resolved eventually with #470.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment