| msg205988 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2013-12-12 23:02 |
| Thanks to the PEP 442 (Safe object finalization), _TracebackLogger helper of asyncio.Futures becomes useless. Attached patch removes it. -- If you agree to diverge code with Tulip project, the Python 3.3 code of WriteTransport.writelines can also be removed: if not PY34: # In Python 3.3, bytes.join() doesn't handle memoryview. list_of_data = ( bytes(data) if isinstance(data, memoryview) else data for data in list_of_data) |
|
|
| msg205990 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2013-12-12 23:46 |
| I'm sorry, I really don't want the asyncio code for 3.3 and 3.4 to diverge just yet. It would make keeping the two versions in sync just so much harder. I'm happy with something that checks for the version and either adds the __del__ method (for 3.4) or uses the _TracebackLogger (for 3.3). |
|
|
| msg205992 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-13 00:15 |
| > I'm happy with something that checks for the version and either adds the __del__ method (for 3.4) or uses the _TracebackLogger (for 3.3). Ok. Here is a patch. The class is still defined on Python 3.4. You may move the definition of the class in a "if not PY34:" class. I did not the change to have a shorter patch. |
|
|
| msg206625 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-19 15:52 |
| Updated patch to address Guido's comments. |
|
|
| msg206627 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2013-12-19 17:47 |
| So the new patch is fine, but I still think it's confusing that the _tb_logger variable has a different type depending on the Python version. If you really don't want to fix this, just go ahead and check in, it's not a blocker. |
|
|
| msg206640 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-19 19:56 |
| "So the new patch is fine, but I still think it's confusing that the _tb_logger variable has a different type depending on the Python version." To be honest, I'm also concerned by this strange variable :-) Here is a new fix which reuses the name used in the first patch (_traceback). |
|
|
| msg206644 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-19 20:22 |
| asyncio_log_traceback-5.patch: new try :-) |
|
|
| msg206648 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2013-12-19 20:36 |
| Looks good. I can fix that long line myself. :-) |
|
|
| msg206652 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-12-19 21:43 |
| New changeset 5e9728ebb1d3 by Victor Stinner in branch 'default': Close #19967: Thanks to the PEP 442, asyncio.Future can use a destructor in http://hg.python.org/cpython/rev/5e9728ebb1d3 |
|
|
| msg206656 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-12-20 01:09 |
| I think this patch is bad and should be reverted. It always calls traceback.format_exception() which is an expensive operation, while the _TracebackLogger takes care to call it only when necessary. |
|
|
| msg206657 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2013-12-20 01:19 |
| Eew. You're right. Sorry I didn't see this. |
|
|
| msg206673 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-20 09:55 |
| > I think this patch is bad and should be reverted. It always calls traceback.format_exception() which is an expensive operation, while the _TracebackLogger takes care to call it only when necessary. Oh, I didn't notice that, and I agree that the new code is inefficient. Since the Future object does not release the reference to the exception after result() or exception() has been called, there is no need to preformat the exception. It can be done in the destructor. Attached asyncio_defer_format_tb.patch implements that. Future.set_exception() creates a reference cycle. I created the issue #20032 to discuss that. |
|
|
| msg206714 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2013-12-20 22:41 |
| Victor, can you commit the fix (with my suggested improvement)? |
|
|
| msg206716 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-12-20 23:20 |
| New changeset 06ed1691efdc by Victor Stinner in branch 'default': Issue #19967: Defer the formating of the traceback in asyncio.Future destructor http://hg.python.org/cpython/rev/06ed1691efdc |
|
|