Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(80)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 810933003: Avoid emitting addStubs and tearOffCode when not needed. (Closed)

Created:
5 years, 11 months ago by zarah
Modified:
5 years, 11 months ago
Reviewers:
ahe, floitsch
CC:
reviews_dartlang.org, ahe
Target Ref:
refs/remotes/git-svn
Visibility:
Public.

Description

Avoid emitting addStubs and tearOffCode when not needed. R=floitsch@google.com Committed: https://code.google.com/p/dart/source/detail?r=42654

Patch Set 1 #

Total comments: 2

Patch Set 2 : Added comment. #

Patch Set 3 : Always emit the functions in incremental compilations. #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -8 lines) Patch
M pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart View 1 chunk +1 line, -1 line 2 comments Download
M pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart View 1 1 chunk +4 lines, -0 lines 4 comments Download
M pkg/compiler/lib/src/js_emitter/old_emitter/reflection_data_parser.dart View 1 5 chunks +13 lines, -6 lines 0 comments Download
M pkg/dart2js_incremental/lib/caching_compiler.dart View 1 2 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 8 (2 generated)
zarah
5 years, 11 months ago (2015-01-05 21:14:00 UTC) #2
floitsch
LGTM. https://codereview.chromium.org/810933003/diff/1/pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right): https://codereview.chromium.org/810933003/diff/1/pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart#newcode27 pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:27: bool needsArrayInitializerSupport = false; Comment what this is.
5 years, 11 months ago (2015-01-06 10:15:13 UTC) #3
zarah
https://codereview.chromium.org/810933003/diff/1/pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right): https://codereview.chromium.org/810933003/diff/1/pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart#newcode27 pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:27: bool needsArrayInitializerSupport = false; On 2015/01/06 10:15:13, floitsch wrote: ...
5 years, 11 months ago (2015-01-07 08:08:22 UTC) #4
zarah
Committed patchset #3 (id:40001) manually as 42654 (presubmit successful).
5 years, 11 months ago (2015-01-07 13:07:50 UTC) #5
ahe
https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart File pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart (right): https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart#newcode455 pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart:455: emitter.needsArrayInitializerSupport = true; I'd call this "needsStructuredMemberInfo". https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart File ...
5 years, 11 months ago (2015-01-07 13:14:52 UTC) #7
zarah
5 years, 11 months ago (2015-01-07 14:23:09 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart (right):

https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/container_builder.dart:455:
emitter.needsArrayInitializerSupport = true;
On 2015/01/07 13:14:52, ahe wrote:
> I'd call this "needsStructuredMemberInfo".

Done, in another CL.

https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart (right):

https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:27: /// This is set to
true in ContainerBuilder if the program contains
On 2015/01/07 13:14:52, ahe wrote:
> Add newline before line 27 and after line 30.

Done, in another CL.

https://codereview.chromium.org/810933003/diff/40001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart:28: /// function
elements that need extra handling. In this case the element is
On 2015/01/07 13:14:52, ahe wrote:
> This is a rather long comment, but it actually doesn't contain much
information.
> 
> The part "extra handling" is vague, but also what is interesting.
> 
> I'd suggest:
> 
> /// True if [ContainerBuilder.addMemberMethodFromInfo] used "structured info",
> that is, some function was needed for reflection, had stubs, or had a super
> alias.
> 
> This uses the same wording and terminology as used in
> ContainerBuilder.addMemberMethodFromInfo.

Done, in another CL.

Powered by Google App Engine
This is Rietveld 408576698