[issue42093] Add opcode cache for LOAD_ATTR

[STINNER Victor]

STINNER Victor  added the comment:


New changeset d5fc99873769f0d0d5c5d5d99059177a75a4e46e by Victor Stinner in 
branch 'master':
bpo-42093: Cleanup _PyDict_GetItemHint() (GH-24582)
https://github.com/python/cpython/commit/d5fc99873769f0d0d5c5d5d99059177a75a4e46e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner
nosy_count: 4.0 -> 5.0
pull_requests: +23361
pull_request: https://github.com/python/cpython/pull/24582

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Guido van Rossum]

Guido van Rossum  added the comment:

Hm, I was thinking to recognize the specific type of descriptor used by slots 
and cache only that. Though we would still have to consider updates to 
C.__dict__ (that's handled by looking at the dict version right?).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> Thanks! Do you have any plans for further inline caches?

Yeah, we are experimenting with some ideas here: 
https://bugs.python.org/issue42115. 

>  I was wondering if we could reverse the situation for slots again by adding 
> slots support to the LOAD_ATTR opcode inline cache...

I think we can do it as long as we can detect easily if a given descriptor is 
immutable. The problem of mutability is this code:

class Descriptor:
pass

class C:
def __init__(self):
self.x = 1
x = Descriptor()

def f(o):
return o.x

o = C()
for i in range(1):
assert f(o) == 1

Descriptor.__get__ = lambda self, instance, value: 2
Descriptor.__set__ = lambda *args: None

print(f(o))

In this case, if we do not skip the cache for mutable descriptors, the code 
will not reflect the new result (2 instead of 1). __slots__ are immutable 
descriptors so we should be good as long as we can detect them.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Guido van Rossum]

Guido van Rossum  added the comment:

Thanks! Do you have any plans for further inline caches? I was wondering if we 
could reverse the situation for slots again by adding slots support to the 
LOAD_ATTR opcode inline cache...

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 9e8fe1986cb4205fb9f883c89b9d5d76a9847e0b by Pablo Galindo in 
branch 'master':
bpo-42093: Tweak the what's new message about the new LOAD_ATTR opcode cache 
(GH-24070)
https://github.com/python/cpython/commit/9e8fe1986cb4205fb9f883c89b9d5d76a9847e0b


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +22903
pull_request: https://github.com/python/cpython/pull/24070

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

>Wow, this is amazing. I just found that this is now faster than slots. 

Not only that: is even faster than the highly-tuned namedtuple access 
descriptors that used to be the faster access to an attribute:

3.9 results
---
  17.6 ns   read_classvar_from_class
  16.3 ns   read_classvar_from_instance
  23.2 ns   read_instancevar
  19.7 ns   read_instancevar_slots
  17.9 ns   read_namedtuple
  39.2 ns   read_boundmethod

Now this is the faster way to get an attribute:

3.10 results

  17.9 ns   read_classvar_from_class
  16.9 ns   read_classvar_from_instance
  14.1 ns   read_instancevar
  20.0 ns   read_instancevar_slots
  18.0 ns   read_namedtuple
  40.7 ns   read_boundmethod

> Should we mention that in What's New?

Good idea!. I will prepare a PR complementing the current paragraph.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Guido van Rossum]

Guido van Rossum  added the comment:

Wow, this is amazing. I just found that this is now faster than slots. Should 
we mention that in What's New?

(Of course there's an optimization possible for slots as well, but it would 
require complicating the cache struct. Maybe in 3.11. :-)

--
nosy: +Guido.van.Rossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 109826c8508dd02e06ae0f1784f1d202495a8680 by Pablo Galindo in 
branch 'master':
bpo-42093: Add opcode cache for LOAD_ATTR (GH-22803)
https://github.com/python/cpython/commit/109826c8508dd02e06ae0f1784f1d202495a8680


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
nosy: +yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
keywords: +patch
pull_requests: +21760
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/22803

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42093] Add opcode cache for LOAD_ATTR

[Pablo Galindo Salgado]

New submission from Pablo Galindo Salgado :

>From the creators of "opcode cache for LOAD_GLOBAL" 
>(https://bugs.python.org/issue26219) now it's time for "opcode cache for 
>LOAD_ATTR: the revenge". This issue/PR builds on top of Yury's original patch 
>in the same way https://bugs.python.org/issue26219 did for LOAD_GLOBAL. 

These are the benchmark results for the pyperformance test suite with 
PGO/LTO/CPU ISOLATION in a tuned system with pyperf:

+-+--+-+
| Benchmark   | 2020-10-20_01-18-master-de73d432bb29 | 
2020-10-20_02-28-cache-68f931f6938a |
+=+==+=+
| go  | 407 ms   | 349 ms: 
1.17x faster (-14%) |
+-+--+-+
| raytrace| 822 ms   | 730 ms: 
1.13x faster (-11%) |
+-+--+-+
| unpickle_pure_python| 497 us   | 447 us: 
1.11x faster (-10%) |
+-+--+-+
| scimark_sor | 311 ms   | 280 ms: 
1.11x faster (-10%) |
+-+--+-+
| hexiom  | 15.4 ms  | 14.0 ms: 
1.10x faster (-9%) |
+-+--+-+
| logging_silent  | 302 ns   | 276 ns: 
1.10x faster (-9%)  |
+-+--+-+
| chaos   | 176 ms   | 163 ms: 
1.08x faster (-7%)  |
+-+--+-+
| pyflate | 1.01 sec | 948 ms: 
1.06x faster (-6%)  |
+-+--+-+
| scimark_lu  | 246 ms   | 232 ms: 
1.06x faster (-6%)  |
+-+--+-+
| pickle_pure_python  | 712 us   | 674 us: 
1.06x faster (-5%)  |
+-+--+-+
| regex_effbot| 4.49 ms  | 4.26 ms: 
1.05x faster (-5%) |
+-+--+-+
| scimark_monte_carlo | 160 ms   | 153 ms: 
1.05x faster (-5%)  |
+-+--+-+
| richards| 120 ms   | 115 ms: 
1.05x faster (-4%)  |
+-+--+-+
| 2to3| 458 ms   | 442 ms: 
1.04x faster (-4%)  |
+-+--+-+
| regex_v8| 33.7 ms  | 32.5 ms: 
1.04x faster (-3%) |
+-+--+-+
| scimark_sparse_mat_mult | 7.16 ms  | 6.93 ms: 
1.03x faster (-3%) |
+-+--+-+
| deltablue   | 12.1 ms  | 11.7 ms: 
1.03x faster (-3%) |
+-+--+-+
| regex_dna   | 268 ms   | 261 ms: 
1.03x faster (-3%)  |
+-+--+-+
| meteor_contest  | 152 ms   | 148 ms: 
1.03x faster (-3%)  |
+-+--+-+
| genshi_xml  | 89.0 ms  | 87.1 ms: 
1.02x faster (-2%) |
+-+--+-+
|