wip #528
wip #528
Conversation
| int accepted_fd; \ | ||
| int fd; \ | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Grep for UV_LOOP_PRIVATE_PLATFORM_FIELDS and use that approach instead. Duplicating the fields is bound to break later on.
|
Lots of remarks but the concept seems sound enough to me. :-) |
| @@ -175,6 +175,24 @@ struct uv__io_s { | |||
| uv_handle_t* next_closing; \ | |||
|
|
|||
|
|
|||
| #if defined(__APPLE__) | |||
|
|
|||
| # define UV_STREAM_PRIVATE_PLATFORM_FIELDS \ | |||
There was a problem hiding this comment.
doesn't this waste a big chunk of memory per stream on os x?
There was a problem hiding this comment.
you're right, I can allocate it on-demand
| uv_thread_t thread; | ||
| uv_sem_t sem; | ||
| uv_mutex_t mutex; | ||
| struct uv_async_s* async; |
There was a problem hiding this comment.
It's unknown at the moment of declaration of this structure
There was a problem hiding this comment.
It's not possible to shuffle a little bit?
There was a problem hiding this comment.
Fedor, you can turn the field in UV_STREAM_PRIVATE_PLATFORM_FIELDS into a void* and move the declaration of uv__stream_select_s into src/unix/stream.c.
|
Everything resolved. |
| uv_thread_t thread; | ||
| uv_sem_t sem; | ||
| uv_mutex_t mutex; | ||
| uv_async_t* async; |
There was a problem hiding this comment.
Why don't you embed the uv_async_t?
|
Fixed everything. |
|
This pull request passes (merged 1b1b1da9 into e22af9ba). |
| } | ||
|
|
||
|
|
||
| int uv__stream_needs_select(uv_stream_t* stream, int fd) { |
There was a problem hiding this comment.
Convention: return -1 on error, 0 on success.
There was a problem hiding this comment.
Well, I need to rename it then.
kqueue(2) on osx doesn't work (emits EINVAL error) with specific fds (i.e. /dev/tty, /dev/null, etc). When given such descriptors - start select(2) watcher thread that will emit io events.
|
Does it looks better now? |
|
Oh, pushed into another repo :) Should be good now. |
|
Thanks Fedor, landed in 5da380a. |
wip