-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
src: switch from QUEUE to intrusive list #667
Conversation
/cc @misterdjules, refs nodejs/node-v0.x-archive#9098. |
@@ -47,7 +45,7 @@ inline AsyncWrap::AsyncWrap(Environment* env, | |||
if (try_catch.HasCaught()) | |||
FatalError("node::AsyncWrap::AsyncWrap", "init hook threw"); | |||
|
|||
has_async_queue_ = true; | |||
bits_ |= 1; // has_async_queue() is true now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic constant? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it's not super pretty. Once I find a few more spots like this, I'll introduce a BitField class.
Few nits, otherwise LGTM |
@indutny Fixed the debug agent, PTAL. |
LGTM |
a485e9d
to
b445108
Compare
Waiting to see if https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/134/ works around VS's weak parser. |
@piscisaureus Can you PTAL and try to figure out why MSVS keeps choking on it? |
@bnoordhuis Played with it for an hour, I have no idea what the problem is. |
@bnoordhuis possible solution: piscisaureus@a6714b0 |
Yes, I think it is a MSVC Bug https://connect.microsoft.com/VisualStudio/SearchResults.aspx?SearchQuery=C3855 |
8d4e5fc
to
0a37a43
Compare
@bnoordhuis |
@piscisaureus gcc complains that a bare |
@bnoordhuis sounds like #ifdef galore |
0a37a43
to
ff9c3a5
Compare
I'm afraid so... let's see what the CI thinks: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/144/ |
@piscisaureus Can you either LGTM this PR or come up with some suggestions? :-) |
This is a replacement for the QUEUE macros. It implements the same functionality but in a way that lets the compiler typecheck it. PR-URL: nodejs#667 Reviewed-By: Bert Belder <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
This commit also breaks up req_wrap.h into req-wrap.h and req-wrap-inl.h to work around a circular dependency issue in env.h. PR-URL: nodejs#667 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
It has been obsoleted by the previous commit. Now it's time to say goodbye. PR-URL: nodejs#667 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
Fold two integral fields into one and use bitops to access/manipulate them. PR-URL: nodejs#667 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
ff9c3a5
to
470d0e5
Compare
Make that https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/167/, the previous one ran into the 'too many open files' issue again on some of the buildbots. |
I'm not having much luck tonight... https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/168/ |
470d0e5
to
4bb3184
Compare
R=@indutny
https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/133/