[issue34551] Redundant store can be removed from _PyFunction_FastCallDict

2018-08-30 Thread Eric Lippert


New submission from Eric Lippert :

In _PyFunction_FastCallDict we have local nk assigned to be the size of a 
dictionary, and then local i is assigned to twice the size of the same 
dictionary, and then nk is assigned to half of i, which it already is:

   nk = (kwargs != NULL) ? PyDict_GET_SIZE(kwargs) : 0;
   if (nk != 0) {
 ...
 pos = i = 0;
 while (PyDict_Next(kwargs, &pos, &k[i], &k[i+1])) {
   ...
   i += 2;
 }
 nk = i / 2;

I am attempting to understand the performance characteristics of this hot path, 
and I spent far too long trying to figure out why nk was being assigned a value 
it already has. :)

I propose that the redundant store be replaced with an assertion that i/2 is 
equal to nk.

I will submit a pull request presently.

--
components: Interpreter Core
messages: 324395
nosy: Eric Lippert
priority: normal
severity: normal
status: open
title: Redundant store can be removed from _PyFunction_FastCallDict
type: enhancement
versions: Python 3.8

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



[issue34551] Redundant store can be removed from _PyFunction_FastCallDict

2018-08-30 Thread Eric Lippert


Change by Eric Lippert :


--
keywords: +patch
pull_requests: +8479
stage:  -> patch review

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



[issue34551] Redundant store can be removed from _PyFunction_FastCallDict

2018-08-31 Thread Eric Lippert


Eric Lippert  added the comment:

If it were possible that the interpreter iterating over a dictionary could 
cause the dictionary to change size then I suspect that this would be a rich 
source of bugs to mine. :-) 

It would be great if we could make it a documented, enforced invariant that the 
interpreter reading a dictionary never produces a side effect visible to the 
interpreter.

Re: "this specific statement already existed before my work."

Thanks for the historical note; the first thing I thought when I read this line 
was "that's got to be a copy-paste left over from an earlier version of the 
algorithm."

--

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