Extract computing UnlinkedUnit(s) and cache them in ByteStore.

Extract computing UnlinkedUnit(s) and cache them in ByteStore.

So, we usually don't have to parse most of the files in order to
compute imports/exports/parts and API signatures.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/8925c68a7182505542f056a1ac6d9435ecdfd5c9

[Paul Berry, 2017-06-13 21:49:46]

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:25: new
TopLevelParser(listener).parseUnit(token);
It seems wasteful that we call `parseUnit` here and on line 29.  Could we
instead modify _BodySkippingParser so that it records directives?

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:61: void
_excludeNativeClauses(Token token) {
This seems like an unnecessarily fragile hack.  Can't we rely on the existing
parser support for native clauses?

[scheglov, 2017-06-13 22:03:21]

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:25: new
TopLevelParser(listener).parseUnit(token);
On 2017/06/13 21:49:46, Paul Berry wrote:
> It seems wasteful that we call `parseUnit` here and on line 29.  Could we
> instead modify _BodySkippingParser so that it records directives?

Well, yes, we waste something. But in general TopLevelParser in very cheap - it
does not parse class members and bodies. I'd like to land it as is and then make
some measurements for how long it takes to parse here and below. If it is
expensive, we will look how to optimize it.

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:61: void
_excludeNativeClauses(Token token) {
On 2017/06/13 21:49:46, Paul Berry wrote:
> This seems like an unnecessarily fragile hack.  Can't we rely on the existing
> parser support for native clauses?

Unfortunately my understanding is that position of Fasta is that native clauses
are not a part of the language, so they are not supported. It requires handling
[codeExpectedFunctionBody] in listener. I thought that for my usecase this rough
handling of native clauses is good enough.

[Paul Berry, 2017-06-14 12:20:52]

lgtm

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:61: void
_excludeNativeClauses(Token token) {
On 2017/06/13 22:03:21, scheglov wrote:
> On 2017/06/13 21:49:46, Paul Berry wrote:
> > This seems like an unnecessarily fragile hack.  Can't we rely on the
existing
> > parser support for native clauses?
> 
> Unfortunately my understanding is that position of Fasta is that native
clauses
> are not a part of the language, so they are not supported. It requires
handling
> [codeExpectedFunctionBody] in listener. I thought that for my usecase this
rough
> handling of native clauses is good enough.

Ok, I will have a discussion with Peter about this.

[scheglov, 2017-06-14 15:10:01]

Description was changed from

==========
Extract computing UnlinkedUnit(s) and cache them in ByteStore.

So, we usually don't have to parse most of the files in order to
compute imports/exports/parts and API signatures.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Extract computing UnlinkedUnit(s) and cache them in ByteStore.

So, we usually don't have to parse most of the files in order to
compute imports/exports/parts and API signatures.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/8925c68a7182505542f056a1ac6d9435ecdfd5c9
==========

[scheglov, 2017-06-14 15:10:02]

Committed patchset #1 (id:1) manually as
8925c68a7182505542f056a1ac6d9435ecdfd5c9 (presubmit successful).

[Siggi Cherem (dart-lang), 2017-06-14 22:37:38]

dbc

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:61: void
_excludeNativeClauses(Token token) {
On 2017/06/14 12:20:52, Paul Berry wrote:
> On 2017/06/13 22:03:21, scheglov wrote:
> > On 2017/06/13 21:49:46, Paul Berry wrote:
> > > This seems like an unnecessarily fragile hack.  Can't we rely on the
> existing
> > > parser support for native clauses?
> > 
> > Unfortunately my understanding is that position of Fasta is that native
> clauses
> > are not a part of the language, so they are not supported. It requires
> handling
> > [codeExpectedFunctionBody] in listener. I thought that for my usecase this
> rough
> > handling of native clauses is good enough.
> 
> Ok, I will have a discussion with Peter about this.

FWIW - in our perf scripts we take the other approach:
https://github.com/dart-lang/sdk/blob/master/pkg/front_end/tool/fasta_perf.da...
(add logic in the directive-listener to handle native clauses).

It is handled only for the VM, and doing something more general requires that we
delegate to a target-implementation like we do in the regular compiles in fasta

[scheglov, 2017-06-14 23:20:34]

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:61: void
_excludeNativeClauses(Token token) {
On 2017/06/14 22:37:38, Siggi Cherem (dart-lang) wrote:
> On 2017/06/14 12:20:52, Paul Berry wrote:
> > On 2017/06/13 22:03:21, scheglov wrote:
> > > On 2017/06/13 21:49:46, Paul Berry wrote:
> > > > This seems like an unnecessarily fragile hack.  Can't we rely on the
> > existing
> > > > parser support for native clauses?
> > > 
> > > Unfortunately my understanding is that position of Fasta is that native
> > clauses
> > > are not a part of the language, so they are not supported. It requires
> > handling
> > > [codeExpectedFunctionBody] in listener. I thought that for my usecase this
> > rough
> > > handling of native clauses is good enough.
> > 
> > Ok, I will have a discussion with Peter about this.
> 
> FWIW - in our perf scripts we take the other approach:
>
https://github.com/dart-lang/sdk/blob/master/pkg/front_end/tool/fasta_perf.da...
> (add logic in the directive-listener to handle native clauses).
> 
> It is handled only for the VM, and doing something more general requires that
we
> delegate to a target-implementation like we do in the regular compiles in
fasta

OK, I see how this is done for parsing directives.
Does not quite work for parsing and skipping bodies.
I've opened an issue against Fasta / Parser.
https://github.com/dart-lang/sdk/issues/29879

[ahe, 2017-08-07 14:05:13]

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/unlinked_unit.dart (right):

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:113: Token
parseFunctionBody(Token token, bool isExpression, bool allowAbstract) {
This feature is already provided by ClassMemberParser. If it doesn't do exactly
what you need, I think we should extend it.

Generally, it would be great if we could avoid subclassing Parser outside
pkg/front_end/lib/src/fasta/parser/. This makes it easier to make parser
changes.

https://codereview.chromium.org/2939823002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/unlinked_unit.dart:122: Token
parseMixinApplication(Token token) {
You should be able to collect this information via a listener.