-
Notifications
You must be signed in to change notification settings - Fork 30k
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: minor worker code cleanups #32562
Conversation
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: nodejs#32344
There’s no reason for this to not be stored alongside the loop data structure itself.
w->loop_init_failed_ = true; | ||
w->thread_joined_ = true; | ||
w->stopped_ = true; | ||
w->env()->remove_sub_worker_context(w); |
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.
Why these assignments are removed ?
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.
@HarshithaKP Because they either had no effect or were just un-setting variables that were set above.
LGTM |
Landed in 0c70e8c...f558a76 |
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: #32344 PR-URL: #32562 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
There’s no reason for this to not be stored alongside the loop data structure itself. PR-URL: #32562 Refs: #32344 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: #32344 PR-URL: #32562 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
There’s no reason for this to not be stored alongside the loop data structure itself. PR-URL: #32562 Refs: #32344 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: #32344 PR-URL: #32562 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
There’s no reason for this to not be stored alongside the loop data structure itself. PR-URL: #32562 Refs: #32344 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: nodejs#32344 PR-URL: nodejs#32562 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
There’s no reason for this to not be stored alongside the loop data structure itself. PR-URL: nodejs#32562 Refs: nodejs#32344 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
Instead of setting and then in the case of error un-setting properties, only set them when no error occurs. Refs: #32344 PR-URL: #32562 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
There’s no reason for this to not be stored alongside the loop data structure itself. PR-URL: #32562 Refs: #32344 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: James M Snell <[email protected]>
src: clean up worker thread creation code
Instead of setting and then in the case of error un-setting properties,
only set them when no error occurs.
Refs: #32344
src: remove loop_init_failed_ from Worker class
There’s no reason for this to not be stored alongside the loop
data structure itself.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes