gh-92810: Reduce memory usage by ABCMeta.__subclasscheck__#131914
gh-92810: Reduce memory usage by ABCMeta.__subclasscheck__#131914dolfinus wants to merge 46 commits into
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
3 similar comments
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Signed-off-by: Martynov Maxim <martinov_m_s_@mail.ru>
Signed-off-by: Martynov Maxim <martinov_m_s_@mail.ru>
Signed-off-by: Martynov Maxim <martinov_m_s_@mail.ru>
Signed-off-by: Martynov Maxim <martinov_m_s_@mail.ru>
Signed-off-by: Martynov Maxim <martinov_m_s_@mail.ru>
abf4bfe to
b7603e0
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
I've seen your work but I really don't have enough time to estimate the impact for all cases. It's not that it's a bad change but
|
|
Thanks @picnixz, I understand your point. There is no rush here, the change can be landed any time. |
+1 That sounds good. The PR with new tests can be merged as is and then we can continue working on this ensure that the behavior remains same. |
|
Extracted tests into #144941 |
|
@picnixz gentle ping about this and related PRs |
|
I'm sorry but as I said, I don't feel confident enough to accept the change now and I would prefer that other core devs express their opinions. Now, AFAIR, and AFAICT, most common cases are faster and consume less memory right? Does this change actually affect performance when we try to do instance/subclass checks for totally unrelated classes? (in the presented benchmarks, there is a common ancestor, but what if we have two entirely different trees, is there anything slowing down those paths?) |
|
Ok, I'll add this case in benchmark a bit later. What do you think about #144941? It does not contain any changes in abc module source code, only new tests. |
|
This PR is stale because it has been open for 30 days with no activity. |
Documentation build overview
|
Added this test case to benchmark, timing and RAM usage remains the same. Also resolved conflicts with main branch. |
|
Note that there are 2 other implementations:
|
_abc._abc_subclasscheckhas very poor performance and (I think) a memory leak #92810For python build using
--enable-optimizations:benchmark.py
isinstance(child, Parent)4MiB...15MiB
4MiB...15MiB
11MiB...24MiB
10MiB...24MiB
issubclass(Child, Parent)0MiB...1MiB
0MiB...1MiB
6MiB...8MiB
5MiB...8MiB
isinstance(child, Grandparent)0MiB...2MiB
0MiB...1MiB
4MiB...7MiB
4MiB...7MiB
issubclass(Child, Grandparent)0MiB
0MiB
0MiB...1MiB
0MiB...1MiB
not isinstance(child, Sibling)4MiB...14MiB
2MiB...14MiB
13MiB...23MiB
13MiB...22MiB
not issubclass(Child, Sibling)1MiB
0MiB...1MiB
8MiB...10MiB
8MiB...11MiB
not isinstance(child, Cousin)1MiB...2MiB
1MiB
7MiB...9MiB
7MiB...9MiB
not issubclass(Child, Cousin)0MiB
0MiB...1MiB
4MiB
3MiB...4MiB
not isinstance(child, Uncle)6174MiB...6333MiB
0MiB...1MiB
4382MiB...4422MiB
6MiB
not issubclass(Child, Uncle)6171MiB
0MiB
4380MiB
4MiB
Memory increment is measured during
isinstance()/issubclass()calls, not during preparation, like class creation or registration where actual registry allocation is performed. So memory usage in tables below is almost always 0.Timing drop for 2 first rows is mostly due to
cls._abc_cache.add(scls)call withindef register(slc, scls)which wasn't implemented before.isinstance(child, Parent.register)0MiB
0MiB
0MiB
0MiB
issubclass(Child, Parent.register)0MiB
0MiB
0MiB
0MiB
isinstance(child, Grandparent.register)0MiB
0MiB
0MiB
0MiB
issubclass(Child, Grandparent.register)0MiB
0MiB
0MiB
0MiB
not isinstance(child, Sibling.register)0MiB
1MiB
0MiB
2MiB
not issubclass(Child, Sibling.register)0MiB
1MiB
0MiB
2MiB
not isinstance(child, Cousin.register)0MiB
2MiB
0MiB
3MiB
not issubclass(Child, Cousin.register)0MiB
2MiB
0MiB
3MiB
not isinstance(child, Uncle.register)0MiB
2MiB...3MiB
0MiB
4MiB
not issubclass(Child, Uncle.register)0MiB
2MiB
0MiB
4MiB
This became a bit slower due to new checks, but this is rare case.
isinstance(child, Parent.__subclasses__)0MiB
0MiB
0MiB
0MiB
issubclass(Child, Parent.__subclasses__)0MiB
0MiB
0MiB
0MiB
isinstance(child, Grandparent.__subclasses__)0MiB
0MiB
0MiB
0MiB
issubclass(Child, Grandparent.__subclasses__)0MiB
0MiB
0MiB
0MiB
not isinstance(child, Sibling.__subclasses__)0MiB
0MiB
0MiB
1MiB
not issubclass(Child, Sibling.__subclasses__)0MiB
0MiB
0MiB
1MiB
not isinstance(child, Cousin.__subclasses__)0MiB
0MiB
0MiB
1MiB
not issubclass(Child, Cousin.__subclasses__)0MiB
0MiB
0MiB
1MiB
not isinstance(child, Uncle.__subclasses__)0MiB
1MiB
0MiB
2MiB
not issubclass(Child, Uncle.__subclasses__)0MiB
0MiB
0MiB
2MiB
Just to check that nothing is broken:
not isinstance(child, Unrelated)0MiB
0MiB
0MiB
0MiB
not issubclass(Child, Unrelated)0MiB
0MiB
0MiB
0MiB
not isinstance(child, UnrelatedABC)0MiB
0MiB
0MiB
0MiB
not issubclass(Child, UnrelatedABC)0MiB
0MiB
0MiB
0MiB
Flamegraphs for
_py_abcimpl and testissubclass_uncle(the most time and memory consuming case onmain):main_vs_pr131914.tar.gz
Alternatives:
__subclasscheck__to subclass creation, more multithread-friendly