Skip to content

Conversation

@pitrou
Copy link
Contributor

@pitrou pitrou commented Dec 29, 2017

A suspended coroutine should be GCed if the underlying loop is closed and no other outside reference exists to it. However, a suspended coroutine with a refcycle would be kept alive by the _futures_to_runners mapping. Instead use a private attribute on the future.

A suspended coroutine should be GCed if the underlying loop is closed
and no other outside reference exists to it.  However, a suspended coroutine
with a refcycle would be kept alive by the _futures_to_runners mapping.
Instead use a private attribute on the future.
@ploxiln
Copy link
Contributor

ploxiln commented Dec 31, 2017

Do you think it's just bad luck (travis-ci server load) that every build configurations of https://travis-ci.org/tornadoweb/tornado/builds/323627058 took more than twice as long as usual to run?

@pitrou
Copy link
Contributor Author

pitrou commented Dec 31, 2017

@ploxiln I just noticed that. It seems Travis-CI is saturated these days. When I run the test suite locally, I don't get any runtime difference on the test suite between this PR and git master (17 seconds on both).

Even pip install Twisted seems to take a full minute:
https://travis-ci.org/tornadoweb/tornado/jobs/323627063#L461

Perhaps @bdarnell can restart the build at some point to see if runtimes vary. I don't have the access rights to do so.

@pitrou
Copy link
Contributor Author

pitrou commented Dec 31, 2017

Now we're even getting "import timed out" errors:
https://travis-ci.org/tornadoweb/tornado/jobs/323627065#L1133

@ploxiln
Copy link
Contributor

ploxiln commented Dec 31, 2017

OK yeah it's just travis, nevermind me :)

@pitrou
Copy link
Contributor Author

pitrou commented Dec 31, 2017

FTR, builds have also become awfully long on CPython.

@bdarnell
Copy link
Member

Thanks for this. I've got it rebased and squashed in #2248 with the change to AsyncIOMainLoop as a separate commit. This commit produced some log warnings at GC time that I spent some time hunting down.

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.

3 participants