Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Speedup from switch to +=

github.com

Post reply on HN
Print view View original

41–50 of 85 posts

Re: Speedup from switch to +=

#41

Post by WirelessGigabit »

thomasahle wrote: post #37
JonathonW wrote: post #7

If they're seeing these kinds of gains from relatively minor changes to their Python code, I can't help but wonder how much faster the model would run in a compiled language or a language with a good JIT (way more optimization work's gone into the mainstream Javascript runtimes than CPython). I'd assumed that overall performance in Stable Diffusion was limited by the code running on the GPU, with Python performance b…

In PyTorch `x = y + x` is actually semantically different from `x += y`, so you can't easily make the switch with a compiler. The difference is that `x += y` modifies `x` inplace, where `x = x + y` creates a new object. In other words, if anybody had a reference to `x` before the update, the "optimized" code would break things.

Compiler could use a pointer to pointer.

I guess this is the kind of this stuff that drew me to Rust. This kind of behavior gives me the creeps. Just like Ruby’s conventions.

Re: Speedup from switch to +=

#42

Post by staticassertion »

FabHK wrote: post #39

Plot twist: it breaks the code...? > Changing this back to the original implementation fixed an error I was getting when doing textual inversion on Windows https://github.com/lstein/stable-diffusion/commit/62863ac586...

Love to see it. A perfect example of why this optimization can't be done automatically - in the case of `else` you're working with a mutable reference to `x` passed in, which means that now your function is mutating something it used to not mutate.

A "safe" way to do this is still straightforward, I think.

    from copy import copy
    def _forward(self, x, context=None):
        x = x.contiguous() if x.device.type == 'mps' else x
        x = copy(x)
        x += self.attn1(self.norm1(x))
        x += self.attn2(self.norm2(x), context=context)
        x += self.ff(self.norm3(x))
        return x
It could be faster but I don't know what `x` is and I'm not going to guess. Also, `copy` may not be sufficient, `deepcopy` may be necessary - again, I don't know what `x` is so I can't figure that out. Pls use type annotations :)

Re: Speedup from switch to +=

#43

Post by masklinn »

jonas21 wrote: post #20
JonathonW wrote: post #7

If they're seeing these kinds of gains from relatively minor changes to their Python code, I can't help but wonder how much faster the model would run in a compiled language or a language with a good JIT (way more optimization work's gone into the mainstream Javascript runtimes than CPython). I'd assumed that overall performance in Stable Diffusion was limited by the code running on the GPU, with Python performance b…

This is PyTorch code, so the Python is setting up a bunch of kernels that are executed on the GPU. The switch from + to += might allow two of those kernels to be fused together or something, and that could lead to the large performance gain. The Python part only runs a handful of times so JIT vs. non-JIT doesn't really make a difference.

Nah it’s because PyTorch has a different implementation for __iadd__. It’s saving a copy by mutating the LHS in-place, and possibly more divergent as comments report broken code.

Re: Speedup from switch to +=

#44

Post by mgraczyk »

staticassertion wrote: post #42
FabHK wrote: post #39

Plot twist: it breaks the code...? > Changing this back to the original implementation fixed an error I was getting when doing textual inversion on Windows https://github.com/lstein/stable-diffusion/commit/62863ac586...

Love to see it. A perfect example of why this optimization can't be done automatically - in the case of `else` you're working with a mutable reference to `x` passed in, which means that now your function is mutating something it used to not mutate. A "safe" way to do this is still straightforward, I think. from copy import copy def _forward(self, x, context=None): x = x.contiguous() if x.device.type == 'mps' else x x…

That's not safe if the problem is the in place mutation. You will still mutate x while reading from it.

Re: Speedup from switch to +=

#45

Post by masklinn »

staticassertion wrote: post #34
ironhaven wrote: post #24

Because of operator overloading "+=" can call a more optimized method than "+". If this code was written in a language without operator overloading I don't think this would be a very interesting pull request. THis could be a example of why some people don't like operator overloading and why some programing languages (java, zig, etc) do not implment the feature.

I don't think this is an operator overloading thing? It's just that `x = y + x` is equivalent to z = y + x x = z Basically, creating an object `z` just to throw it away. `x += y` just adds y to x directly without any intermediary. You could write this in any language pretty easily. For example, in Rust: let x = "abc".to_string(); let y = "123".to_string(); let x = x + &y; as opposed to the more efficient: let mut x =…

> I don't think this is an operator overloading thing?

It’s the confusion / idea that this is trivial change which is the overload thing.

Re: Speedup from switch to +=

#46

Post by NavinF »

ironhaven wrote: post #24

Because of operator overloading "+=" can call a more optimized method than "+". If this code was written in a language without operator overloading I don't think this would be a very interesting pull request. THis could be a example of why some people don't like operator overloading and why some programing languages (java, zig, etc) do not implment the feature.

Operator overloading is a major reason why libraries like pytorch exist so IMO that's a moot point.

Btw there's ongoing work to automatically optimize expressions like this. See the XLA compiler for example. Right now deep learning has a ton of seemingly obvious compute/memory optimisations that are not done automatically.

Re: Speedup from switch to +=

#48

Post by NavinF »

Waterluvian wrote: post #14

One comment asks about putting it all on one line, and this is where interpreted languages without a JIT kinda blow. Many times I have had to decide if my Python code would be more legible or get free performance. The thing I like about JavaScript is that I can _usually_ trust the JIT to make my code faster than I could, meaning I can focus entirely on writing clean code. P.S. you can always hand optimize. If you do,…

This has nothing to do with python. A JITed/AoT compiled version of the old code should do exactly the same thing because it would build the same pytorch graph.

Re: Speedup from switch to +=

#49

Post by desmond373 »

mgraczyk wrote: post #44
staticassertion wrote: post #42

Earlier quoted context omitted.

Love to see it. A perfect example of why this optimization can't be done automatically - in the case of `else` you're working with a mutable reference to `x` passed in, which means that now your function is mutating something it used to not mutate. A "safe" way to do this is still straightforward, I think. from copy import copy def _forward(self, x, context=None): x = x.contiguous() if x.device.type == 'mps' else x x…

That's not safe if the problem is the in place mutation. You will still mutate x while reading from it.

Might be platform dependent whether that first line counts as a mutate or not, seeing as it can be converted to not do anything in some cases.

Re: Speedup from switch to +=

#50

Post by mgraczyk »

desmond373 wrote: post #49
mgraczyk wrote: post #44

Earlier quoted context omitted.

That's not safe if the problem is the in place mutation. You will still mutate x while reading from it.

Might be platform dependent whether that first line counts as a mutate or not, seeing as it can be converted to not do anything in some cases.

All of the lines that have += mutate x in place
Post reply on HN