Skip to content
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

Merged
merged 4 commits into from
Feb 11, 2015

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis
Copy link
Member Author

/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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic constant? :)

Copy link
Member Author

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.

@indutny
Copy link
Member

indutny commented Jan 30, 2015

Few nits, otherwise LGTM

@bnoordhuis
Copy link
Member Author

@indutny Fixed the debug agent, PTAL.

@indutny
Copy link
Member

indutny commented Jan 30, 2015

LGTM

@bnoordhuis
Copy link
Member Author

Waiting to see if https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/134/ works around VS's weak parser.

@bnoordhuis
Copy link
Member Author

@piscisaureus Can you PTAL and try to figure out why MSVS keeps choking on it?

@piscisaureus
Copy link
Contributor

@bnoordhuis Played with it for an hour, I have no idea what the problem is.

@piscisaureus
Copy link
Contributor

@bnoordhuis possible solution: piscisaureus@a6714b0
With that the other fixup commits can be omitted.

@mathiask88
Copy link
Contributor

@piscisaureus
Copy link
Contributor

@bnoordhuis
That won't work. Don't do ListNodeMember<T>, just use template <typename T, ListNodeMember M>

@bnoordhuis
Copy link
Member Author

@piscisaureus gcc complains that a bare ListNodeMember isn't a type - which is more or less right because it's a generic type, not a concrete type...

@piscisaureus
Copy link
Contributor

@bnoordhuis sounds like #ifdef galore

@bnoordhuis
Copy link
Member Author

I'm afraid so... let's see what the CI thinks: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/144/

@bnoordhuis
Copy link
Member Author

@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 <bertbelder@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
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 <fedor.indutny@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
It has been obsoleted by the previous commit.  Now it's time to say
goodbye.

PR-URL: nodejs#667
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fold two integral fields into one and use bitops to access/manipulate
them.

PR-URL: nodejs#667
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@bnoordhuis
Copy link
Member Author

@bnoordhuis
Copy link
Member Author

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.

@bnoordhuis
Copy link
Member Author

I'm not having much luck tonight... https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/168/

@bnoordhuis bnoordhuis merged commit 4bb3184 into nodejs:v1.x Feb 11, 2015
@bnoordhuis bnoordhuis deleted the typesafe-intrusive-list branch February 11, 2015 23:23
@rvagg rvagg mentioned this pull request Feb 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants