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

71–80 of 85 posts

Re: Speedup from switch to +=

#71

Post by codeflo »

fragmede wrote: post #66
WirelessGigabit wrote: post #41

Earlier quoted context omitted.

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.

Rust has the same behavior. https://news.ycombinator.com/item?id=32805756

Not to turn this into an evangelism subthread, but I think GP is right. The performance optimization could apply, but the bug as described should be prevented by the borrow checker. In fact you can do neither x = … nor x += … when anything else still references x, because assignment requires an exclusive reference (&mut), which is statistically checked.

Re: Speedup from switch to +=

#72

Post by crabbycarrot »

umvi wrote: post #65
nodja wrote: post #62

I see lots of people answering why it's faster, but not many saying why the engineers chose the slower version. As everyone said, this is more performant because x is being modified in place, the reason this was not done in place is because you can't train a neural network if an instruction is being done in place. During training a network goes literally through all operations that were done and see how well they per…

See, this is a great example of where a comment needed to be added, but wasn't. If the engineers that originally implemented the function intentionally chose the slower version, a quick comment as to why would have prevented this from happening in the first place.

On ML teams, a comment like this would not get past code review because it's obvious – avoiding in-place operations in PyTorch is the standard.

Re: Speedup from switch to +=

#73

Post by joelgibson »

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 change isn't a matter of Python being slower than a compiled language, it's changing the meaning of the code. The line

  x = x + y
creates a copy of the array x, adds y to it, and then sets the variable x to that new array. In contrast, the line

  x += y
adds the array y in-place into the array x (and so hopefully no other piece of code is relying on x being immutable). This kind of trade-off occurs in pretty much all programming, for instance you see it whenever big-integer libraries are used in C++ or Rust.

Re: Speedup from switch to +=

#74

Post by FabHK »

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…

How about this? (As the copy operation implicit in x=x+y seemed ok.)

    def _forward(self, x, context=None):
        x = x.contiguous() if x.device.type == 'mps' else x
        x = x + self.attn1(self.norm1(x))
        x += self.attn2(self.norm2(x), context=context)
        x += self.ff(self.norm3(x))
        return x

Re: Speedup from switch to +=

#75

Post by _flux »

fragmede wrote: post #66
WirelessGigabit wrote: post #41

Earlier quoted context omitted.

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.

Rust has the same behavior. https://news.ycombinator.com/item?id=32805756

I'm not sure what you mean by "same behavior". I don't think you can externally distinguish between "x = x + x" and "x += x" in Rust, because while in the first case a new temporary value will be created, it will overwrite the left side value: x will be the "same object" as it's in the same address in both cases. In Python objects are stored indirectly, so after an assignment x will then point to a new object instead of the old one.

But yes, not making copies is more efficient so the same optimization applies.

Nevertheless, in Rust (or C++) the parameter x would not have been passed as a mutable reference into the function, so the value provided as input would not have been mutated.

Re: Speedup from switch to +=

#76

Post by FabHK »

mgraczyk wrote: post #59
Arnavion wrote: post #51

Earlier quoted context omitted.

staticassertion's point is that the current code's usage of `+=` mutates the x that was passed in by the caller, and their suggestion is to copy x into a function local before mutating it, which is similar to how the original `+` code also worked on a function local x (the result of `attn1() + x`).

That's not the problem though. The problem is that the += operations mutate x in place, but the right hand side reads from x. There is no copy you could insert like that to fix this. You would have to do the following, for example Instead of x = op(x) + x -> x += op(x) Do x_copy = copy(x) x += op(x_copy) If you do x_copy += op(x_copy) Then you are still mutating x_copy while op() reads it. EDIT: I also don't think co…

In x += op(x)

mutation of LHS x really starts before RHS has been evaluated completely?

Re: Speedup from switch to +=

#77

Post by mgraczyk »

FabHK wrote: post #76
mgraczyk wrote: post #59

Earlier quoted context omitted.

That's not the problem though. The problem is that the += operations mutate x in place, but the right hand side reads from x. There is no copy you could insert like that to fix this. You would have to do the following, for example Instead of x = op(x) + x -> x += op(x) Do x_copy = copy(x) x += op(x_copy) If you do x_copy += op(x_copy) Then you are still mutating x_copy while op() reads it. EDIT: I also don't think co…

In x += op(x) mutation of LHS x really starts before RHS has been evaluated completely?

I assume so, although if I remember correctly the actual error in the thread is caused by the the gradient computation

Re: Speedup from switch to +=

#78

Post by codeflo »

thayne wrote: post #26
sshine wrote: post #18

Earlier quoted context omitted.

I don't know anything about stable diffusion, but I've been optimizing a lot of prime-field arithmetic in Rust lately, and we experienced a similar speedup going from `+ x` to `+= x` (for scalars and especially for composite structures like vectors and polynomials).

For composite structures that isn't too surprising, but for scalars, I would have expected llvm to optimize the addition and assignment into a single in place addition.

The short answer is yes.

The long answer is that it’s not so clear what an “in place addition” even means at the level of CPU instructions after you consider register allocation. For example, if you have

    v = x;
    v += y;
    f(v);
and the never mention v again, then the whole operation is performed directly in the register that is specified to receive the first argument in a function call, not in whatever register might have been allocated for v.

That’s because, with some complications I don’t want to go into, compilers look at the dependency graph of values rather than at the variable names.

Re: Speedup from switch to +=

#79

Post by Someone »

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…

Alternatively, keep the first line as is. That gives you a copy that’s only known to the function, so you can change the later ones.

I would only do that if I had seen it to be faster, though, and add a comment on why the first line couldn’t do +=.

Re: Speedup from switch to +=

#80

Post by chillee »

Ok, I work on PyTorch, so probably should clear up some misconceptions in this thread.

1. In PyTorch (and other array programming libraries like Numpy), the operations being passed around are tensors/arrays (i.e. large chunks of memory). Thus, += is overloaded to mean "in-place write" to the arrays.

So, `+` vs `+=` is the equivalent of

    a: float[1000]
    b: float[1000]
    for i in [0, 1000]:
        b[i] = a[i] + 2
vs.

    a: float[1000]
    for i in [0, 1000]:
        a[i] = a[i] + 2
The main performance advantage comes in 1. no need to allocate an extra array, 2. you're using less memory overall, so various caching levels can work better. It has nothing to do with python bytecodes.

2. As for whether it generally makes sense to do this optimization manually... Usually, PyTorch users don't use in-place operations as its a bit uglier mathematically and have various foot-guns/restrictions that users find confusing. Generally, it's best to have this optimization be done automatically by an optimizing compiler.

3. PyTorch in general does support using in-place operations during training, albeit with some caveats.

(PS) 4. Putting everything on one line (as some folks suggest) is almost certainly not going to help performance - the primary performance bottlenecks here have almost nothing to do with CPU perf.

Post reply on HN