5710 initial implementation of using posix_spawn #1675
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…six_spawn isn't used when testing the other code path
for more information, see https://pre-commit.ci
…pawn - does this need fixing?
for more information, see https://pre-commit.ci
glyph
changed the title
|
One follow-up I can think of here is making the usage of posix_spawn a bit more visible. It might be a huge performance boost for some applications, and those should be able to figure out what it was that made a difference. |
| @@ -643,6 +677,7 @@ def __init__( | |||
| nuances of setXXuid on UNIX: it will assume that either your effective | |||
| or real UID is 0.) | |||
| """ | |||
| self._reactor = reactor | |||
There was a problem hiding this comment.
Rather than hanging a new attribute on the object, perhaps just pass this along to _fork and then _trySpawnInsteadOfFork?
There was a problem hiding this comment.
I think that having _reactor class/instance member is ok.
It's a common patter and makes testing much easier.
The _trySpawnInsteadOfFork is private API, so I think that doing a dependency injection there is more complicated.
| @@ -643,6 +677,7 @@ def __init__( | |||
| nuances of setXXuid on UNIX: it will assume that either your effective | |||
| or real UID is 0.) | |||
| """ | |||
| self._reactor = reactor | |||
There was a problem hiding this comment.
I think that having _reactor class/instance member is ok.
It's a common patter and makes testing much easier.
The _trySpawnInsteadOfFork is private API, so I think that doing a dependency injection there is more complicated.
Scope and purpose
Add a few words about why this PR is needed and what is its scope.
Add any comments about trade-offs (if any) made in this PR and the reasoning behind them.
Add mentions of things that are not covered here and are planed to be done in separate PRs.
Contributor Checklist:
tox -e lintto format my patch to meet the Twisted Coding Standard#character).reviewto the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.The first line is automatically generated by GitHub based on PR ID and branch name.
The other lines generated by GitHub should be replaced.