msg199553 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-12 12:03 |
As discussed on python-dev, importing _decimal at the bottom of
decimal.py is about 9x slower than importing _decimal directly.
|
msg199555 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-10-12 12:21 |
I proposed something similar for issue #19229.
|
msg199558 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2013-10-12 12:43 |
Remember that one reason for importing the C version at the bottom of the python version is so that alternate implementations (PyPy, IronPython, Jython) could provide partial versions of the C (or equivalent) versions. By importing after the Python version, the alternate implementation could continue to use parts of the Python code.
I think the impact on alternate implementations needs to be considered before we start rearchitecting these imports.
|
msg199562 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-12 13:00 |
Right, let's start collecting objections. :)
Mark, Raymond: Would you support the change (__name__ hack and all)?
Maciej: Is this approach a problem for PyPy?
|
msg199564 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-10-12 13:08 |
If the Python implementation is renamed to _pydecimal, I don't expect it to be used in CPython. I never used _pyio in a real application, only for some tests to debug. I don't think that we need the __name__ = 'decimal' "hack". If you really want to keep it, please add at least a comment explaining it.
|
msg199567 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-12 13:20 |
I guess if some of the pickling stuff get's rewritten, we can drop
__name__.
The other thing is that traditionally the types were "decimal.Decimal"
etc., so I'm not sure if it is good idea to have "_decimal.Decimal" and
"_pydecimal.Decimal".
Of course adding __module__ everywhere is another option.
|
msg199570 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-10-12 13:24 |
> The other thing is that traditionally the types were "decimal.Decimal"
> etc., so I'm not sure if it is good idea to have "_decimal.Decimal" and
> "_pydecimal.Decimal".
Why not renaming the _decimal module to decimal?
|
msg199571 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-12 13:32 |
_decimal already lies about its name (for pickling).
|
msg199612 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-12 18:47 |
I can't apply the patch that was created with diff --git, so here is
another one that is less readable but applies.
|
msg199619 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-12 20:34 |
> I can't apply the patch that was created with diff --git, so here is
> another one that is less readable but applies.
You can apply it using "hg import --no-commit", I think.
|
msg199677 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-10-13 08:59 |
> Mark, Raymond: Would you support the change (__name__ hack and all)?
No objections here.
|
msg199691 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-13 10:59 |
Antoine Pitrou <report@bugs.python.org> wrote:
> You can apply it using "hg import --no-commit", I think.
mercurial 2.1 throws an exception even though the patch was created with
that version. Now I upgraded to 2.7.2 and it works.
Rietveld also seems to choke on the first patch (no review link).
|
msg199724 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-10-13 15:35 |
I think we should save these sort of tricks only for modules imported during startup.
Ideally, a user should expect that the code for the decimal module is in decimal.py. Ideally, tools like IDLE's "Open Module" should be able to find the source code using only the module name.
I'm -0 on this one. I don't think hacking up the source tree is worth it. The import is only done once -- the important part is what decimal does when it is imported.
|
msg199864 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-14 11:00 |
About IDLE I can't say anything, but I'm not entirely sure if the
PEP-399 import method is easier to understand for users, see e.g.:
http://stackoverflow.com/questions/13194384/instantiate-decimal-class
I get the impression that the posters at first did not even realize
that there *is* an accelerator in Python 3.3, so they had a hard time
debugging the issue.
As for the load time: Personally I don't have any application
that relies on a very short load time, but I wan't to note again
that importing decimal at Python startup actually doubles the
startup time.
So if there are applications that start a new Python script
for each request (e.g. via djb's tcpserver), it could really
matter.
OTOH I don't like moving code around either, so we can wait until
there's a demonstrated need for the speedup.
|
msg199872 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-14 11:44 |
> OTOH I don't like moving code around either, so we can wait until
> there's a demonstrated need for the speedup.
Keep in mind that people who have startup speed problems aren't
likely to open an issue on the tracker about it, let alone
diagnose it enough to put the blame on decimal.
|
msg216562 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2014-04-16 19:39 |
I would like to go ahead with this. As Antoine mentioned, most
people don't diagnose import problems, especially when they compare
Python 2 against Python 3.
As it turns out, the slowdown is even significant in a simple tcpserver
application that starts a Python script.
Another benefit is that it will be possible to import the Python version
easily, e.g. to check error messages (currently _decimal justs displays
the signal in tracebacks). I've often wanted that myself instead of
going through the import_fresh_module() ordeal.
|
msg226700 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-09-10 16:01 |
New changeset 8bf51cf94405 by Stefan Krah in branch 'default':
Issue #19232: Speed up decimal import. Additionally, since _decimal is
http://hg.python.org/cpython/rev/8bf51cf94405
|
msg226701 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2014-09-10 17:16 |
We could speed up the import further by not importing collections
in _decimal. That could be done once structseq fully implements
the namedtuple protocol (for DecimalTuple).
|
msg226812 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-09-12 07:43 |
"We could speed up the import further by not importing collections
in _decimal. That could be done once structseq fully implements
the namedtuple protocol (for DecimalTuple)."
I suggest to close this issue. I guess that importing decimal is already fast enough, and enhance structseq is a completly different issue. (Is there an open issue, just to get the link?)
|
msg226816 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2014-09-12 09:55 |
I'm fine with closing this. The structseq issue is #1820.
|
msg229141 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-10-12 11:31 |
New changeset 75b5617b8dfc by Stefan Krah in branch 'default':
Issue #19232: Fix sys.modules lookup (--without-threads)
https://hg.python.org/cpython/rev/75b5617b8dfc
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:51 | admin | set | github: 63431 |
2014-10-12 11:31:14 | python-dev | set | messages:
+ msg229141 |
2014-09-12 09:55:05 | skrah | set | status: open -> closed versions:
+ Python 3.5, - Python 3.4 messages:
+ msg226816
resolution: fixed stage: patch review -> resolved |
2014-09-12 07:43:19 | vstinner | set | messages:
+ msg226812 |
2014-09-10 17:16:23 | skrah | set | messages:
+ msg226701 |
2014-09-10 16:01:20 | python-dev | set | nosy:
+ python-dev messages:
+ msg226700
|
2014-09-09 18:01:35 | skrah | unlink | issue22284 dependencies |
2014-08-27 10:17:04 | skrah | link | issue22284 dependencies |
2014-04-16 19:39:57 | skrah | set | messages:
+ msg216562 |
2014-04-16 16:48:14 | skrah | link | issue21227 dependencies |
2013-10-14 11:44:41 | pitrou | set | messages:
+ msg199872 |
2013-10-14 11:00:01 | skrah | set | messages:
+ msg199864 |
2013-10-13 16:31:42 | Arfrever | set | nosy:
+ Arfrever
|
2013-10-13 15:35:56 | rhettinger | set | messages:
+ msg199724 |
2013-10-13 10:59:01 | skrah | set | messages:
+ msg199691 |
2013-10-13 08:59:23 | mark.dickinson | set | messages:
+ msg199677 |
2013-10-12 20:34:38 | pitrou | set | nosy:
+ pitrou messages:
+ msg199619
|
2013-10-12 18:47:36 | skrah | set | files:
+ issue19232-2.patch
messages:
+ msg199612 |
2013-10-12 15:32:38 | barry | set | nosy:
+ barry
|
2013-10-12 13:32:46 | skrah | set | messages:
+ msg199571 |
2013-10-12 13:24:20 | vstinner | set | messages:
+ msg199570 |
2013-10-12 13:20:30 | skrah | set | messages:
+ msg199567 |
2013-10-12 13:08:07 | vstinner | set | messages:
+ msg199564 |
2013-10-12 13:02:22 | skrah | set | stage: needs patch -> patch review |
2013-10-12 13:00:25 | skrah | set | nosy:
+ rhettinger, mark.dickinson, fijall messages:
+ msg199562
|
2013-10-12 12:49:02 | skrah | set | files:
+ issue19232.patch keywords:
+ patch |
2013-10-12 12:43:41 | eric.smith | set | nosy:
+ eric.smith messages:
+ msg199558
|
2013-10-12 12:21:39 | vstinner | set | messages:
+ msg199555 |
2013-10-12 12:21:12 | vstinner | set | nosy:
+ vstinner
|
2013-10-12 12:03:19 | skrah | create | |