Skip to content

Conversation

@zdevito
Copy link
Contributor

@zdevito zdevito commented Jun 1, 2019

Reduce the number of stack manipulation instructions by finding places where we can operate directly on the stack.

Stack from ghstack:

For TestScript, this reduces the total number of instructions emitted across the test suite from 50k to 37k.

I also used the following script:

import torch

@torch.jit.script
def fib(x):
    # type: (int) -> int
    prev = 1
    v = 1
    for i in range(0, x):
        save = v
        v = v + prev
        prev = save
        # print(v)

    #print("Done")
    return v



fib(10000000)

To time the interpreter overhead using the new approach compared to the old approach. Both performed with no measurable performance differences, suggesting the overheads lie not in the interpretation but in other aspects (e.g. refcounting, std::function invocation).

BEFORE CHANGE

real	0m0.689s
user	0m0.630s
sys	0m0.045s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.708s
user	0m0.645s
sys	0m0.060s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.732s
user	0m0.668s
sys	0m0.052s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.730s
user	0m0.653s
sys	0m0.054s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.729s
user	0m0.662s
sys	0m0.052s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.732s
user	0m0.667s
sys	0m0.057s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.721s
user	0m0.653s
sys	0m0.054s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py

real	0m0.696s
user	0m0.633s
sys	0m0.049s


-------------------------------

AFTER CHANGE

real	0m0.665s
user	0m0.616s
sys	0m0.048s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py
nums = 81

real	0m0.661s
user	0m0.594s
sys	0m0.053s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py
nums = 81

real	0m0.662s
user	0m0.592s
sys	0m0.053s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py
nums = 81

real	0m0.676s
user	0m0.608s
sys	0m0.048s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py
nums = 81

real	0m0.662s
user	0m0.587s
sys	0m0.062s
(py3) [[email protected] /data/users/zdevito/pytorch] time python what.py
nums = 81

real	0m0.642s
user	0m0.578s
sys	0m0.050s
```

Differential Revision: [D15590900](https://our.internmc.facebook.com/intern/diff/D15590900)

@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jun 1, 2019
@zdevito zdevito changed the title unfinished push/pop reduction Reduce number of stack manipulation instructions in interpreter. Jun 1, 2019
…reter."

unfinished push/pop reduction

gh-metadata: pytorch pytorch 21240 gh/zdevito/45/head
…reter."

unfinished push/pop reduction

gh-metadata: pytorch pytorch 21240 gh/zdevito/45/head
Copy link
Member

@suo suo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requesting changes to clear my queue, pls re-request when it's ready

…reter."

unfinished push/pop reduction

gh-metadata: pytorch pytorch 21240 gh/zdevito/45/head
@zdevito zdevito requested a review from suo June 5, 2019 05:08
@ezyang ezyang added facebook and removed facebook labels Jun 5, 2019
// and we can short circuit doing many instructions here
// by either clearing the register (DROPR) or just popping the stack
// (DROP)
if (preprocess_.can_emit_inline[input->node()]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment about what the significant of the check to can_emit_inline is here

…reter."

unfinished push/pop reduction

gh-metadata: pytorch pytorch 21240 gh/zdevito/45/head
…reter."

unfinished push/pop reduction

gh-metadata: pytorch pytorch 21240 gh/zdevito/45/head
@zou3519 zou3519 deleted the gh/zdevito/45/head branch June 8, 2019 03:59
@facebook-github-bot
Copy link
Contributor

@zdevito merged this pull request in dde2795.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants