Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Adding uv_break, because sometimes the world is not perfect :-) #676

Closed
wants to merge 2 commits into from

3 participants

@saghul

I commented this a while ago on the mailing list and today I had some hours to kill at an airport so here it is. I couldn't comment on the implementation since I had no Internet when I was writing it, so all comments are welcome :-)

/cc @bnoordhuis @piscisaureus

@bnoordhuis

A libuv user can accomplish the same thing by combining UV_RUN_ONCE or UV_RUN_NOWAIT and a global flag, right? I don't particularly like adding API functions that are easily emulated in a user's code.

@saghul

It can be kind of emulated, but it can be expensive. In pyuv I don't like the idea of inventing APIs libuv doesn't have so I'd have to do the loop in Python, and going back and forth between C and Python has an overhead. This is of course a particularity of how pyuv works, but IMHO uv_break fits in, and I (for one) would take advantage of it :-)

@saghul

Is there anything I can do to convince you this is a nice to have thing? :-)

@txdv

What? I love inventing APIs that libuv doesn't have.
I have coded RunOnce before your run2 patch.

In what scenarios would you use it?

@saghul

pyuv is a shallow wrapper around libuv, not a framework built on top of it. Of course it's adapted to the Python universe/syntax, but right now I don't have any "extra" APIs, and I'd rather stay that way.

I've used pyuv to monkeypatch different frameworks like Twisted or Tornado, and some of these provide 3 functions for a given loop: run, stop and close. Run does the obvious, that is, run the loop until there is nothing left to be done or stop is called, stop just stops the loop and close frees all resources and fds associated with the loop.

So, that would be very easy to implement as follows: run -> uv_run(default), stop -> uv_break, close -> uv_walk + uv_close all handles + uv_run(nowait).

Yes, I can do it directly in Python, but it's a lot slower :-S

Also, since we can run a loop it makes sense to me that we should be able to stop it at will.

Maybe s/uv_break/uv_stop/ ?

@txdv

Yeah, stop would be nice, i'm for this.

@saghul

Hi, it's stubborn @saghul again, last attempt :-)

I wrote a silly example which involves a loop that runs until a idle handle has made a million iterations. In one case it's stopped with loop.stop (I'm thinking that uv_stop looks better that uv_break) and the other has a global variable to do the same trick. Here are the results: https://gist.github.com/4497505

As it can be seen there is a performance penalty in doing the custom loop thing (due to Python being used here), but I don't want to focus on that. IMHO, the version with the run once loop looks a bit weird while the other one is more straightforward and just Looks Better (TM).

Not that it matters much, but similar libraries such as libevent and libev do provide a similar mechanism. The semantics of uv_break are, however, simpler: end the event loop as soon as possible, actually in the next loop iteration. It's up to the user to decide if he wants to let the loop block for io or he wants to start an idle handle to prevent it from blocking.

IMHO this is a convenient API function with a clear meaning and a really simple implementation, so it's not like I'm asking to merge a bunch of crap anyway ;-)

PS: If you guys are willing to merge it, uv_break or uv_stop?

@saghul

Ok, after some more thoughts I simplified the implementation and renamed uv_break to uv_stop. I made an important change in the behavior: if uv_stop is called before blocking for i/o, it will prevent the blocking. The reason is that if you called uv_stop you do want to stop, not wait in case a repeating timer or something would make the loop block for i/o.

Now, with the aforementioned change in semantics implementing it manually becomes trickier because UV_RUN_ONCE would block and UV_RUN_WAIT would prevent blocking for i/o always, so UV_RUN_ONCE would need to be used in combination with an idle handle for example. Still doable, but uv_stop doesn't require an extra handle :-)

@bnoordhuis

@saghul You mention that going back and forth between Python and C land is slow. Can you quantify that?

And when is that an actual issue? I mean, how often do you need to break out of the event loop?

@saghul

@bnoordhuis Thanks for coming back to this, Ben. I was hoping that we could reach some common ground before 0.10.0.

I did a performance benchmark a while ago (https://gist.github.com/4497505), but please just don't look at it. The performance argument is not the most important reason why I want uv_stop.

As for how often do I need to break out of the loop, actually almost always. Twisted and Tornado are the most used event loops in Python and they both have the concept of "running forever" until the user calls stop o the event loop. I wrote pyuv reactors for both frameworks and had to use some sort of trick to break out of run while still leaving the loop in a usable state, because it can be run later again.

The upcoming event loop implementation for the standard library (Tulip) also has this concept.

Now, I could understand why uv_stop wasn't that useful in its first implemented form, because it was nothing more than a flag. The current code up for review, however, takes an extra step and prevents the loop from blocking for i/o on that iteration, which IMHO is a good thing.

I know there is no use for this in Node, but I think it's a useful construct to have. Please, please merge it :-)

@saghul

@bnoordhuis I just rebased the patch with latest master and force pushed.

src/unix/core.c
((8 lines not shown))
uv__update_time(loop);
uv__run_timers(loop);
uv__run_idle(loop);
uv__run_prepare(loop);
uv__run_pending(loop);
- uv__io_poll(loop, (mode & UV_RUN_NOWAIT ? 0 : uv_backend_timeout(loop)));
+ uv__io_poll(loop, ((mode & UV_RUN_NOWAIT ||
+ loop->stop_flag) ? 0 : uv_backend_timeout(loop)));

I'd rather the conditional is split off into one or more separate statements. Ditto for the conditional in src/win/core.c. Otherwise LGTM.

@saghul
saghul added a note

You mean something like this?

int timeout;

...

if (mode & UV_RUN_NOWAIT || loop->stop_flag)
    timeout = 0;
else
    timeout = uv_backend_timeout(loop);
uv__io_poll(loop, timeout);

Yes, but with fixed indentation. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bnoordhuis bnoordhuis closed this
@saghul saghul deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
1  build.mk
@@ -85,6 +85,7 @@ TESTS= \
test/test-ipc.o \
test/test-ipc-send-recv.o \
test/test-loop-handles.o \
+ test/test-loop-stop.o \
test/test-multiple-listen.o \
test/test-mutexes.o \
test/test-pass-always.o \
View
1  include/uv-private/uv-unix.h
@@ -162,7 +162,6 @@ typedef struct {
} uv_lib_t;
#define UV_LOOP_PRIVATE_FIELDS \
- unsigned long flags; \
int backend_fd; \
ngx_queue_t pending_queue; \
ngx_queue_t watcher_queue; \
View
10 include/uv.h
@@ -259,6 +259,14 @@ UV_EXTERN uv_loop_t* uv_default_loop(void);
UV_EXTERN int uv_run(uv_loop_t*, uv_run_mode mode);
/*
+ * This function will stop the event loop by forcing uv_run to end
+ * as soon as possible, but not sooner than the next loop iteration.
+ * If this function was called before blocking for i/o, the loop won't
+ * block for i/o on this iteration.
+ */
+UV_EXTERN void uv_stop(uv_loop_t*);
+
+/*
* Manually modify the event loop's reference count. Useful if the user wants
* to have a handle or timeout that doesn't keep the loop alive.
*/
@@ -1934,6 +1942,8 @@ struct uv_loop_s {
unsigned int active_handles;
ngx_queue_t handle_queue;
ngx_queue_t active_reqs;
+ /* Internal flag to signal loop stop */
+ unsigned int stop_flag;
UV_LOOP_PRIVATE_FIELDS
};
View
13 src/unix/core.c
@@ -280,23 +280,32 @@ static int uv__loop_alive(uv_loop_t* loop) {
int uv_run(uv_loop_t* loop, uv_run_mode mode) {
- int r;
+ int r, timeout;
if (!uv__loop_alive(loop))
return 0;
do {
+ if (loop->stop_flag) {
+ loop->stop_flag = 0;
+ return uv__loop_alive(loop);
+ }
uv__update_time(loop);
uv__run_timers(loop);
uv__run_idle(loop);
uv__run_prepare(loop);
uv__run_pending(loop);
- uv__io_poll(loop, (mode & UV_RUN_NOWAIT ? 0 : uv_backend_timeout(loop)));
+ if (mode & UV_RUN_NOWAIT || loop->stop_flag)
+ timeout = 0;
+ else
+ timeout = uv_backend_timeout(loop);
+ uv__io_poll(loop, timeout);
uv__run_check(loop);
uv__run_closing_handles(loop);
r = uv__loop_alive(loop);
} while (r && !(mode & (UV_RUN_ONCE | UV_RUN_NOWAIT)));
+ loop->stop_flag = 0;
return r;
}
View
1  src/unix/loop.c
@@ -57,6 +57,7 @@ int uv__loop_init(uv_loop_t* loop, int default_loop) {
loop->emfile_fd = -1;
loop->timer_counter = 0;
+ loop->stop_flag = 0;
if (uv__platform_loop_init(loop, default_loop))
return -1;
View
5 src/uv-common.c
@@ -377,3 +377,8 @@ void uv_ref(uv_handle_t* handle) {
void uv_unref(uv_handle_t* handle) {
uv__handle_unref(handle);
}
+
+
+void uv_stop(uv_loop_t* loop) {
+ loop->stop_flag = 1;
+}
View
32 src/win/core.c
@@ -249,10 +249,13 @@ static void uv_poll_ex(uv_loop_t* loop, int block) {
}
}
-#define UV_LOOP_ALIVE(loop) \
- ((loop)->active_handles > 0 || \
- !ngx_queue_empty(&(loop)->active_reqs) || \
- (loop)->endgame_handles != NULL)
+
+static int uv__loop_alive(uv_loop_t* loop) {
+ return loop->active_handles > 0 ||
+ !ngx_queue_empty(&loop->active_reqs) ||
+ loop->endgame_handles != NULL;
+}
+
int uv_run(uv_loop_t *loop, uv_run_mode mode) {
int r;
@@ -263,8 +266,14 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) {
else
poll = &uv_poll;
- r = UV_LOOP_ALIVE(loop);
- while (r) {
+ if (!uv__loop_alive(loop))
+ return 0;
+
+ do {
+ if (loop->stop_flag) {
+ loop->stop_flag = 0;
+ return uv__loop_alive(loop);
+ }
uv_update_time(loop);
uv_process_timers(loop);
@@ -282,14 +291,15 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) {
(*poll)(loop, loop->idle_handles == NULL &&
loop->pending_reqs_tail == NULL &&
loop->endgame_handles == NULL &&
- UV_LOOP_ALIVE(loop) &&
+ !loop->stop_flag &&
+ (loop->active_handles > 0 ||
+ !ngx_queue_empty(&loop->active_reqs)) &&
!(mode & UV_RUN_NOWAIT));
uv_check_invoke(loop);
- r = UV_LOOP_ALIVE(loop);
+ r = uv__loop_alive(loop);
+ } while (r && !(mode & (UV_RUN_ONCE | UV_RUN_NOWAIT)));
- if (mode & (UV_RUN_ONCE | UV_RUN_NOWAIT))
- break;
- }
+ loop->stop_flag = 0;
return r;
}
View
2  test/test-list.h
@@ -23,6 +23,7 @@ TEST_DECLARE (platform_output)
TEST_DECLARE (callback_order)
TEST_DECLARE (run_once)
TEST_DECLARE (run_nowait)
+TEST_DECLARE (loop_stop)
TEST_DECLARE (barrier_1)
TEST_DECLARE (barrier_2)
TEST_DECLARE (barrier_3)
@@ -230,6 +231,7 @@ TASK_LIST_START
#endif
TEST_ENTRY (run_once)
TEST_ENTRY (run_nowait)
+ TEST_ENTRY (loop_stop)
TEST_ENTRY (barrier_1)
TEST_ENTRY (barrier_2)
TEST_ENTRY (barrier_3)
View
55 test/test-loop-stop.c
@@ -0,0 +1,55 @@
+/* Copyright Joyent, Inc. and other Node contributors. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "uv.h"
+#include "task.h"
+
+static uv_timer_t timer_handle;
+static int timer_called = 0;
+static int num_ticks = 10;
+
+
+static void timer_cb(uv_timer_t* handle, int status) {
+ ASSERT(handle == &timer_handle);
+ ASSERT(status == 0);
+ timer_called++;
+ if (timer_called == 1)
+ uv_stop(uv_default_loop());
+ else if (timer_called == num_ticks)
+ uv_timer_stop(handle);
+}
+
+
+TEST_IMPL(loop_stop) {
+ int r;
+ uv_timer_init(uv_default_loop(), &timer_handle);
+ uv_timer_start(&timer_handle, timer_cb, 100, 100);
+
+ r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
+ ASSERT(r != 0);
+ ASSERT(timer_called == 1);
+
+ r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
+ ASSERT(r == 0);
+ ASSERT(timer_called == 10);
+
+ return 0;
+}
View
1  uv.gyp
@@ -280,6 +280,7 @@
'test/test-ipc-send-recv.c',
'test/test-list.h',
'test/test-loop-handles.c',
+ 'test/test-loop-stop.c',
'test/test-walk-handles.c',
'test/test-multiple-listen.c',
'test/test-pass-always.c',
Something went wrong with that request. Please try again.