classification
Title: Implement ABCMeta in C
Type: performance Stage: resolved
Components: Extension Modules, Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Aaron Hall, barry, brett.cannon, eric.snow, gvanrossum, inada.naoki, levkivskyi, miss-islington, ned.deily, scoder, serhiy.storchaka, terry.reedy, vstinner, yselivanov
Priority: deferred blocker Keywords: patch

Created on 2017-09-03 20:54 by levkivskyi, last changed 2018-02-18 22:35 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5273 merged levkivskyi, 2018-01-23 00:36
PR 5733 merged levkivskyi, 2018-02-18 13:29
PR 5744 merged terry.reedy, 2018-02-18 21:37
PR 5745 merged miss-islington, 2018-02-18 21:48
Messages (18)
msg301198 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-09-03 20:54
The idea is that creating ABCs is approximately twice slower than normal classes. Plus there are smaller penalties for various operations with ABCs. This mostly influences the Python interpreter start-up time (because of extensive use of ABCs in importlib), and start-up times of programs that extensively use ABCs.

The situation can be improved by rewriting ABCMeta in C. I have a working implementation, but it is far form being ready and still needs some polishing and optimizations (in particular _abc_cache and friends).

Already at this stage I have one question (I will add more when they appear while I am finishing the implementation): is it OK to make _abc_cache, _abc_negative_cache, _abc_negative_cache_version, and _abc_registry read-only? The point is that I want to prohibit something like this:

    MyABC._abc_cache = "Surprise on updating the cache!"

thus avoiding many PySet_Check(...) calls. These attributes are not documented and start with underscore.
msg301236 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2017-09-04 18:10
> This mostly influences the Python interpreter start-up time
> (because of extensive use of ABCs in importlib)

Just to be clear, the only ABCs in importlib are in importlib.abc (and used by importlib.util).  This does not impact interpreter startup.
msg301241 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-09-04 18:56
Eric,

> the only ABCs in importlib are in importlib.abc (and used by importlib.util).
> This does not impact interpreter startup.

Hm, indeed, but I see that module 'abc' is in 'sys.modules', probably it is then only used by 'collections.abc'/'_collections_abc'. This means that the influence of 'ABCMeta' on interpreter start-up time will be smaller than I thought. But still start-up times for projects that actively use ABCs will be influenced by 'ABCMeta'.

But what do you think about making private ABC caches read-only attributes? (They are not documented)
msg301290 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-09-05 00:25
I've heard many anecdotal complaints about the lack of speed of ABCs. Even if it doesn't affect core Python startup, it does affect startup of apps, and if people are afraid to use them because of this, it's reasonable to try to do something about it.
msg301293 - (view) Author: INADA Naoki (inada.naoki) * (Python committer) Date: 2017-09-05 02:22
> Hm, indeed, but I see that module 'abc' is in 'sys.modules', probably it is then only used by 'collections.abc'/'_collections_abc'. This means that the influence of 'ABCMeta' on interpreter start-up time will be smaller than I thought. But still start-up times for projects that actively use ABCs will be influenced by 'ABCMeta'.

Since os.environ uses _collections_abc.MutableMapping, importing _collections_abc is not avoidable while startup.

`io` module uses ABC and it's used for sys.std(io|err|in).  It's not avoidable too.

And as you know, typing module will be very common rapidly :)
Even if it doesn't affect "python_startup" microbench, it's important.
msg301301 - (view) Author: Stefan Behnel (scoder) * Date: 2017-09-05 08:25
Since the number of applications that get along without any file access is probably close to irrelevant, "os" and "io" feel like sufficiently widely used modules to merit being part of a "usual Python startup" benchmark. Maybe we should add one to the benchmark suite? Anyone up for it?
msg301302 - (view) Author: INADA Naoki (inada.naoki) * (Python committer) Date: 2017-09-05 08:40
"python_startup_nosite" benchmark imports io module,
and "python_startup" benchmark imports os too.
So no need to additional test for them.

You can see it with `python -v -S -c 'pass'` and `python -v -c 'pass'`.
msg310998 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-28 21:25
There has been a lot of work by a number of people on this feature leading up to the 3.7.0b1.  Thank you!  On PR 5273, Yury says that he believes the feature is ready to merge but would prefer an extension until 3.7.0b2 and that Guido has agreed.  I'll defer to Yury's judgement on this and somewhat reluctantly allow an extension: there's a lot going on here.  Let's try to get it in as soon as we can, please!
msg310999 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-28 21:28
> Let's try to get it in as soon as we can, please!

Thank you, Ned!  We'll get it merged in the next few days.
msg312284 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-17 18:02
Isn't 800 lines of C code too high price for speeding up ABCs creation? This will save several microseconds per ABC creation. Even if the program creates 1000 ABCs, this can save just several milliseconds at start-up.
msg312285 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-02-17 18:27
> Isn't 800 lines of C code too high price for speeding up ABCs creation?

800 lines of C code is not something hard to notice, so I suppose the answer is obvious for all people involved in the work on PR :-)

> ...this can save just several milliseconds at start-up.

The correct way to measure this is relative, not absolute. There are just few ABCs used by modules loaded at Python start-up, and it already allowed to save 10% of start-up time. My expectation is that the number will be similar for a typical Python app. Moreover, `isinstance` and `issubclass` (functions called often with ABCs) will be 1.5x faster.
msg312287 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-02-17 19:15
> Isn't 800 lines of C code too high price for speeding up ABCs creation?

I think it's well worth it. This has long felt as a sore point to me.

On Sat, Feb 17, 2018 at 10:27 AM, Ivan Levkivskyi <report@bugs.python.org>
wrote:

>
> Ivan Levkivskyi <levkivskyi@gmail.com> added the comment:
>
> > Isn't 800 lines of C code too high price for speeding up ABCs creation?
>
> 800 lines of C code is not something hard to notice, so I suppose the
> answer is obvious for all people involved in the work on PR :-)
>
> > ...this can save just several milliseconds at start-up.
>
> The correct way to measure this is relative, not absolute. There are just
> few ABCs used by modules loaded at Python start-up, and it already allowed
> to save 10% of start-up time. My expectation is that the number will be
> similar for a typical Python app. Moreover, `isinstance` and `issubclass`
> (functions called often with ABCs) will be 1.5x faster.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue31333>
> _______________________________________
>
msg312306 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-02-18 12:42
New changeset 03e3c340a0156891a036d6dbdb9e348108826255 by Ivan Levkivskyi in branch 'master':
bpo-31333: Re-implement ABCMeta in C (#5273)
https://github.com/python/cpython/commit/03e3c340a0156891a036d6dbdb9e348108826255
msg312315 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-02-18 17:39
New changeset 38928992885d8a04b7188abdba3b04f350bde32d by Ivan Levkivskyi in branch '3.7':
bpo-31333: Re-implement ABCMeta in C (GH-5733)
https://github.com/python/cpython/commit/38928992885d8a04b7188abdba3b04f350bde32d
msg312323 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-02-18 18:51
Congratulations all on this milestone! And thanks reviewers for your thorough work.
msg312327 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-02-18 21:38
PR fixes typo in What's new: rewrittent
msg312329 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-02-18 21:46
New changeset 3fb813d2c67fe28cc98ae51e53a6890294b6e423 by Ivan Levkivskyi (Terry Jan Reedy) in branch 'master':
bpo-31333: Fix typo in whatsnew/3.7.rst (GH-5744)
https://github.com/python/cpython/commit/3fb813d2c67fe28cc98ae51e53a6890294b6e423
msg312331 - (view) Author: miss-islington (miss-islington) Date: 2018-02-18 22:35
New changeset 034a945fa723bf68ca4127bb43bfa5c5be899f17 by Miss Islington (bot) in branch '3.7':
bpo-31333: Fix typo in whatsnew/3.7.rst (GH-5744)
https://github.com/python/cpython/commit/034a945fa723bf68ca4127bb43bfa5c5be899f17
History
Date User Action Args
2018-02-18 22:35:42miss-islingtonsetnosy: + miss-islington
messages: + msg312331
2018-02-18 21:48:06miss-islingtonsetpull_requests: + pull_request5522
2018-02-18 21:46:52levkivskyisetmessages: + msg312329
2018-02-18 21:38:21terry.reedysetnosy: + terry.reedy
messages: + msg312327
2018-02-18 21:37:29terry.reedysetpull_requests: + pull_request5521
2018-02-18 18:51:19gvanrossumsetmessages: + msg312323
2018-02-18 17:39:45levkivskyisetmessages: + msg312315
2018-02-18 13:29:47levkivskyisetpull_requests: + pull_request5514
2018-02-18 12:49:52levkivskyisetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-02-18 12:42:04levkivskyisetmessages: + msg312306
2018-02-17 19:15:01gvanrossumsetmessages: + msg312287
2018-02-17 18:27:40levkivskyisetmessages: + msg312285
2018-02-17 18:02:50serhiy.storchakasetmessages: + msg312284
2018-01-28 21:28:00yselivanovsetmessages: + msg310999
2018-01-28 21:25:50ned.deilysetpriority: normal -> deferred blocker
versions: + Python 3.8
nosy: + ned.deily

messages: + msg310998
2018-01-23 00:36:15levkivskyisetkeywords: + patch
stage: patch review
pull_requests: + pull_request5117
2017-09-05 08:40:37inada.naokisetmessages: + msg301302
2017-09-05 08:25:23scodersetnosy: + scoder
messages: + msg301301
2017-09-05 02:22:15inada.naokisetnosy: + inada.naoki
messages: + msg301293
2017-09-05 00:25:39gvanrossumsetmessages: + msg301290
2017-09-05 00:09:39rhettingersetnosy: + gvanrossum
2017-09-04 18:56:11levkivskyisetmessages: + msg301241
2017-09-04 18:10:24eric.snowsetnosy: + eric.snow
messages: + msg301236
2017-09-04 04:04:41Aaron Hallsetnosy: + Aaron Hall
2017-09-03 20:59:00levkivskyisetnosy: + brett.cannon, vstinner, serhiy.storchaka, yselivanov
2017-09-03 20:54:21levkivskyicreate