[modules] Don't unnecessarily keep function alive after evaluation.

[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569
Committed: https://crrev.com/4c364e63af9b87dd5c2925ddd237e3cf0c29c2ed
Cr-Commit-Position: refs/heads/master@{#40336}

[neis, 2016-10-11 09:38:49]

Let me know what you think. One thing I don't like is that the name 'code' now
makes even less sense.

[adamk, 2016-10-11 15:54:05]

Overall seems reasonable for now (I'm not convinced we won't need to have a
reference to the JSFunction or SharedFunctionInfo again at some point in the
future, but I agree we don't at the moment).

https://codereview.chromium.org/2407183002/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/2407183002/diff/1/src/objects-debug.cc#newcod...
src/objects-debug.cc:937: code()->ObjectVerify();
Maybe just remove all these ObjectVerify calls while you're here? I'm fairly
sure they're redundant.

https://codereview.chromium.org/2407183002/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/2407183002/diff/1/src/objects-inl.h#newcode5720
src/objects-inl.h:5720: DCHECK(!evaluated());
I think I'd prefer killing this method and inlining any callers than leaving it
here, it seems dangerous now.

https://codereview.chromium.org/2407183002/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2407183002/diff/1/src/objects.h#newcode7912
src/objects.h:7912: DECL_ACCESSORS(code, Object)
Some name ideas:

impl
internal
module

[neis, 2016-10-14 09:19:01]

Description was changed from

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates.

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up
the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569
==========

[neis, 2016-10-14 09:20:16]

>
https://codereview.chromium.org/2407183002/diff/1/src/objects-debug.cc#newcod...
> src/objects-debug.cc:937: code()->ObjectVerify();
> Maybe just remove all these ObjectVerify calls while you're here? I'm fairly
> sure they're redundant.

Done.

> https://codereview.chromium.org/2407183002/diff/1/src/objects-inl.h
> File src/objects-inl.h (right):
> 
>
https://codereview.chromium.org/2407183002/diff/1/src/objects-inl.h#newcode5720
> src/objects-inl.h:5720: DCHECK(!evaluated());
> I think I'd prefer killing this method and inlining any callers than leaving
it
> here, it seems dangerous now.

Done.

> https://codereview.chromium.org/2407183002/diff/1/src/objects.h
> File src/objects.h (right):
> 
> https://codereview.chromium.org/2407183002/diff/1/src/objects.h#newcode7912
> src/objects.h:7912: DECL_ACCESSORS(code, Object)
> Some name ideas:
> 
> impl
> internal
> module

I decided to stick with 'code' for now but I clarified its comment.

[neis, 2016-10-14 09:20:39]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 09:20:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 09:51:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 09:51:35]

Dry run: This issue passed the CQ dry run.

[adamk, 2016-10-14 18:01:02]

lgtm

https://codereview.chromium.org/2407183002/diff/60001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2407183002/diff/60001/src/objects.h#newcode8022
src/objects.h:8022: static const int kEmbedderDataOffset =
kRequestedModulesOffset + kPointerSize;
Looks like you'll need a merge here, as embedder data is no more.

[neis, 2016-10-17 06:57:04]

On 2016/10/14 18:01:02, adamk wrote:
> lgtm
> 
> https://codereview.chromium.org/2407183002/diff/60001/src/objects.h
> File src/objects.h (right):
> 
>
https://codereview.chromium.org/2407183002/diff/60001/src/objects.h#newcode8022
> src/objects.h:8022: static const int kEmbedderDataOffset =
> kRequestedModulesOffset + kPointerSize;
> Looks like you'll need a merge here, as embedder data is no more.

We forgot to remove the offset definition. Done now.

[neis, 2016-10-17 07:07:28]

The CQ bit was checked by neis@chromium.org

[neis, 2016-10-17 07:07:29]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407183002/#ps80001
(title: "Remove stale offset.")

[commit-bot: I haz the power, 2016-10-17 07:07:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-17 07:36:48]

Description was changed from

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up
the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up
the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569
==========

[commit-bot: I haz the power, 2016-10-17 07:36:49]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-17 07:37:10]

Description was changed from

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up
the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569
==========

to

==========
[modules] Don't unnecessarily keep function alive after evaluation.

Also hide some implementation details behind abstract predicates and clean up
the heap verifier functions.

R=adamk@chromium.org
BUG=v8:1569

Committed: https://crrev.com/4c364e63af9b87dd5c2925ddd237e3cf0c29c2ed
Cr-Commit-Position: refs/heads/master@{#40336}
==========

[commit-bot: I haz the power, 2016-10-17 07:37:11]

Patchset 5 (id:??) landed as
https://crrev.com/4c364e63af9b87dd5c2925ddd237e3cf0c29c2ed
Cr-Commit-Position: refs/heads/master@{#40336}