[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

Thu Mar 14 11:05:36 PDT 2019

rjmccall added a comment.

In D58514#1429514 <https://reviews.llvm.org/D58514#1429514>, @wuhao5 wrote:

> Hey guys, this is Hao, I am working with Chrome team to sort this issue out.
>
> In D58514#1428606 <https://reviews.llvm.org/D58514#1428606>, @rjmccall wrote:
>
> > I remember this coming up 7-8 years ago.  I intentionally decided against doing a copy/release when assigning to `__weak` because if the block wasn't already guaranteed to be copied then it was probably better to crash than to silently assign a value that's about to be deallocated.  Note that copying the block we assign into `wb` doesn't change anything about the block stored in `b`.
> >
> > I don't know why Chromium is building a weak reference to a block in the first place, but assuming they have a good reason to be doing it, they should fix their code to force a copy before forming a weak reference.
>
>
> The culprit code is here <https://github.com/google/eDistantObject/blob/0146bd6e1f057160bb80e596c644414e05267e9e/Service/Sources/EDOHostService.m#L295>
>
> it's true that it can be copied before assigning to the weak var, and I understand the reasoning behind. however, my question is: just from the code itself, each variable has the proper scope and assignment, if the block copy happen automatically, just like what we should expect ARC would do, should it not mutate itself to something else. to be more precise, should the block assigned to the weak var be the same after the block is copied? (and in the code, the block should be moved to the heap after calling -addObject: a few line below.)
>
> so in the end of day, as a user, should we expect the compiler would move the block from stack to heap in time and the variable we hold is consistent?

The specified user model of blocks is that they stay on the stack until they get copied, and there can be multiple such copies.  ARC just automates that process.  So the address of a block is not consistent before you've forced a copy.

I tend to agree that a better user model would have been for blocks to be allocated in one place, without any of this copying business, and for the compiler to make an intelligent decision about stack vs. heap based on how the block is used.  That's the model we've used for closures in Swift.  But that  would've required the compiler to have a better ability to propagate information about how the block was used, which Clang isn't really set up for, and it would've required `noescape` annotations to be introduced and used reliably throughout the SDK, which seemed like a big request at the time.  So it's not how it works.

There is no way in the existing ABI for copying a block to cause other references to the block to become references to the heap block.  We do do that for `__block` variables, but not for block objects themselves.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58514/new/

https://reviews.llvm.org/D58514