Issue 11414255: dart2js: Dot-separated field-and-super descriptor string in class definitions.

Issue 11414255: dart2js: Dot-separated field-and-super descriptor string in class definitions. (Closed)

Created:
8 years ago by erikcorry

Modified:
8 years ago

Reviewers:
karlklose, ngeoffray

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

dart2js: Dot-separated field-and-super descriptor string in class definitions. This is good for about 1.5% in uncompressed and 0.5% in compressed output. BUG= Committed: https://code.google.com/p/dart/source/detail?r=15577

Patch Set 1 #

Total comments: 10

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+54 lines, -40 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart	View	6 chunks	+44 lines, -34 lines	8 comments	Download
M	sdk/lib/_internal/compiler/implementation/js_backend/emitter_no_eval.dart	View	1 chunk	+3 lines, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/js_backend/native_emitter.dart	View	2 chunks	+7 lines, -5 lines	2 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

karlklose

LGTM/ https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right): https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart#newcode143 sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:143: // defineClass("A", ["x", "y"], { Shouldn't this be ...

8 years ago (2012-11-30 11:43:34 UTC) #1

erikcorry

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right): https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart#newcode143 sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:143: // defineClass("A", ["x", "y"], { On 2012/11/30 11:43:34, karlklose ...

8 years ago (2012-11-30 12:05:57 UTC) #2

ngeoffray

DBC https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right): https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart#newcode143 sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:143: // defineClass("A", ["x", "y"], { So where is ...

8 years ago (2012-11-30 12:11:46 UTC) #3

erikcorry

8 years ago (2012-11-30 12:20:01 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:143: //
defineClass("A", ["x", "y"], {
On 2012/11/30 12:11:46, ngeoffray wrote:
> So where is B in defineClass now?

We don't need the superclass here, so it's nowhere.

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:253: if
(!member) continue;  '''/* if (member == '') */'''
On 2012/11/30 12:11:46, ngeoffray wrote:
> What is this comment about?

The comment shows the intent, the actual code does the same, but is more
compact.

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:833: bool
emitEndingComma: false}) {
On 2012/11/30 12:11:46, ngeoffray wrote:
> Please don't put emitEndingComma optional. Other methods that have the same
> parameter don't have it optional and I prefer to keep it consistent.

I'll revert this in a later CL.

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/native_emitter.dart
(right):

https://codereview.chromium.org/11414255/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/native_emitter.dart:101: if
(method) {  """/* method != '' */"""
On 2012/11/30 12:11:46, ngeoffray wrote:
> What is this about?

As above.

Expand Messages | Collapse Messages