[heap] don't flush async functions, they may be resumed later

[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org, ulan@chromium.org

Committed: https://crrev.com/239309fc8f53b46c76ab1622642f0d2039ef3ddd
Cr-Commit-Position: refs/heads/master@{#36405}

[caitp (gmail), 2016-05-19 16:12:29]

Description was changed from

==========
Reland [test] add tests for async function stacktraces

BUG=v8:4483
R=littledan@chromium.org
==========

to

==========
[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org,
ulan@chromium.org
==========

[caitp (gmail), 2016-05-19 16:12:30]

caitpotter88@gmail.com changed reviewers:
+ hpayer@chromium.org, mstarzinger@chromium.org, ulan@chromium.org

[caitp (gmail), 2016-05-19 16:13:21]

PTAL ---  adds fix for the heap crash from
https://codereview.chromium.org/1997453004

[Dan Ehrenberg, 2016-05-19 17:21:22]

Are there any other calls to is_generator() that may need to be updated?

Seems probably good but I will leave it for the GC folks to review. These are
basically also generators from the GC's perspective.

[caitp (gmail), 2016-05-19 17:30:41]

On 2016/05/19 17:21:22, Dan Ehrenberg wrote:
> Are there any other calls to is_generator() that may need to be updated?
> 

There are a few, which look like they will probably be bugs that will be
addressed when we add more debugger tests.

There are some other things related to picking the [Prototype] to use for new
functions and stuff, but AFAIK these are all tested and doing the right thing,
so
it's probably fine. Here's a quick audit:

```
// IsGeneratorFunction() --- do we want IsAsyncFunction() in the API?
src//api.cc:  return func->shared()->is_generator();

// v8:4280 stuff --- related to deopt support for generators I guess? this
probably needs to work for async functions too
src//compiler.cc:  if (function->shared()->is_generator()) {
src//compiler.cc:    if (shared->is_generator()) return false;

// These look like good candidates for `is_resumable()`
src//debug/debug.cc:    bool include_generators = !is_interpreted &&
shared->is_generator();
src//debug/liveedit.cc:      if (shared->is_generator()) {

// Suspicious, but I think these are all doing the right thing?
src//factory.cc:  if (!function->shared()->is_generator()) {
src//factory.cc:  if (function->shared()->is_generator()) {
src//factory.cc:  if (!function->shared()->is_generator()) {

// Probably needs an update
src//objects-printer.cc:  if (shared()->is_generator()) {

// Function.prototype.toString() stuff --- you already added async support for
these
src//objects.cc:      if (shared_info->is_generator()) {
src//objects.cc:      if (shared_info->is_generator()) {

// Most, if not all of these, are fine
src//parsing/parser-base.h:    bool is_generator() const { return
IsGeneratorFunction(kind_); }
src//parsing/parser-base.h:    bool is_resumable() const { return is_generator()
|| is_async_function(); }
src//parsing/parser-base.h:  bool is_generator() const { return
function_state_->is_generator(); }
src//parsing/parser-base.h:    return
ParseIdentifierOrStrictReservedWord(this->is_generator(),
src//parsing/parser-base.h:              (next == Token::YIELD &&
!is_generator()))) {
src//parsing/parser-base.h:    if (Token::IsIdentifier(name_token,
language_mode(), this->is_generator(),
src//parsing/parser-base.h:  if (peek() == Token::YIELD && is_generator()) {
src//parsing/parser-base.h:      if (!is_generator()) {
src//parsing/parser.cc:  if (is_generator()) {

// Spec issue: can classes extend async functions? they're not constructible, so
we probably should throw for
// async functions too. I don't believe this is specified, though.
src//runtime/runtime-classes.cc:         
Handle<JSFunction>::cast(super_class)->shared()->is_generator()) {
```

> Seems probably good but I will leave it for the GC folks to review. These are
> basically also generators from the GC's perspective.

Yeah, waiting on src/heap owners :>

[Hannes Payer (out of office), 2016-05-20 07:13:52]

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
src/heap/objects-visiting-inl.h:650: // We do not (yet?) flush code for
generator functions, or async functions,
How about a list of items enumerating what we don't flush with a reason next to
it?

[caitp (gmail), 2016-05-20 07:55:48]

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
src/heap/objects-visiting-inl.h:650: // We do not (yet?) flush code for
generator functions, or async functions,
On 2016/05/20 07:13:52, Hannes Payer wrote:
> How about a list of items enumerating what we don't flush with a reason next
to
> it?

isn't that what this routine is? thats how it looks to me. Reorganizing it might
be a job for someone else

[Dan Ehrenberg, 2016-05-20 11:57:41]

lgtm

Async functions are generators from the GC's perspective, and from many other
parts of the compiler. They should be treated as such. Most code should check
is_resumable(). I'm looking at some of those other cases that you mentioned, but
let's get this change in as a start, especially with the instability that it
causes.

[Hannes Payer (out of office), 2016-05-20 12:00:41]

heap owner LGTM

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/1992093003/diff/20001/src/heap/objects-visiti...
src/heap/objects-visiting-inl.h:650: // We do not (yet?) flush code for
generator functions, or async functions,
On 2016/05/20 at 07:55:48, caitp wrote:
> On 2016/05/20 07:13:52, Hannes Payer wrote:
> > How about a list of items enumerating what we don't flush with a reason next
to
> > it?
> 
> isn't that what this routine is? thats how it looks to me. Reorganizing it
might be a job for someone else

Ah, async is based on generators. Then the comments makes sense.

[Dan Ehrenberg, 2016-05-20 12:01:52]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2016-05-20 12:02:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992093003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992093003/20001

[commit-bot: I haz the power, 2016-05-20 12:04:04]

Description was changed from

==========
[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org,
ulan@chromium.org
==========

to

==========
[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org,
ulan@chromium.org
==========

[commit-bot: I haz the power, 2016-05-20 12:04:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-20 12:08:05]

Description was changed from

==========
[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org,
ulan@chromium.org
==========

to

==========
[heap] don't flush async functions, they may be resumed later

BUG=v8:4483
R=littledan@chromium.org, hpayer@chromium.org, mstarzinger@chromium.org,
ulan@chromium.org

Committed: https://crrev.com/239309fc8f53b46c76ab1622642f0d2039ef3ddd
Cr-Commit-Position: refs/heads/master@{#36405}
==========

[commit-bot: I haz the power, 2016-05-20 12:08:06]

Patchset 2 (id:??) landed as
https://crrev.com/239309fc8f53b46c76ab1622642f0d2039ef3ddd
Cr-Commit-Position: refs/heads/master@{#36405}