Skip to content

TST: add temporary elision testcases #4856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 10, 2014

Conversation

juliantaylor
Copy link
Contributor

A very significant optimization for numpy would be to avoid the
temporaries in these types of expressions:

res = a + b + c 

transforming it into:

res = a + b 
res += c

An approach to do that is to check the reference count of the PyNumber
slot arguments, temporary expressions coming from python have reference
count 1 and can be converted to inplace operations.
Unfortunately C-extensions can skip increasing reference counts for
operations breaking the assumption that an inplace operation can be
performed, e.g. Cython does this type of optimizations.
To ensure to not accidentally break such extensions in future test
that refcount == 1 operands from extensions are not elided.

@juliantaylor
Copy link
Contributor Author

see #4322 for a branch where the tests would fail on.

@charris
Copy link
Member

charris commented Jul 9, 2014

Not sure what this is about, could you explain a bit more?

A very significant optimization for numpy would be to avoid the
temporaries in these types of expressions:

    res = a + b + c

transforming it into:

    res = a + b
    res += c

An approach to do that is to check the reference count of the PyNumber
slot arguments, temporary expressions coming from python have reference
count 1 and can be converted to inplace operations.
Unfortunately C-extensions can skip increasing reference counts for
operations breaking the assumption that an inplace operation can be
performed, e.g. Cython does this type of optimizations.
To ensure to not accidentally break such extensions in future test
that refcount == 1 operands from extensions are not elided.
@juliantaylor
Copy link
Contributor Author

updated the commit message

@charris
Copy link
Member

charris commented Jul 9, 2014

OK, makes sense. Is test failure the best way to check this?

@juliantaylor
Copy link
Contributor Author

how else would you test it?

@charris
Copy link
Member

charris commented Jul 10, 2014

I was wondering if you wanted to collect statistics or check if numpy would
fail. I know what is checked, but not sure of the use.
On Jul 9, 2014 6:28 PM, "Julian Taylor" notifications@github.com wrote:

how else would you test it?


Reply to this email directly or view it on GitHub
#4856 (comment).

@juliantaylor
Copy link
Contributor Author

the use is the same as any other test, make sure that something that works now keeps working in the future.
e.g. now we don't need @njsmith good knowledge of pythons inner workings anymore to stop people trying to use the stack scanning approach, the tests will fail if you try it.

charris added a commit that referenced this pull request Jul 10, 2014
TST: add temporary elision testcases
@charris charris merged commit 56b5f27 into numpy:master Jul 10, 2014
@charris
Copy link
Member

charris commented Jul 10, 2014

OK, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants