Sort the output (for now just the classes and instance members).

Sort the output (for now just the classes and instance members).

Committed: https://code.google.com/p/dart/source/detail?r=7447

[floitsch, 2012-05-07 17:06:52]

rebased.

[floitsch, 2012-05-07 20:14:31]

With this CL code emission of Swarm takes now 14secs. So I will have to look if
there are ways to make it faster...
For now you can probably ignore the CL.

[ngeoffray, 2012-05-08 07:58:02]

No surprise this slows down sooooooo much the emitter :) Could we instead used
the natural order provided by the scanner to avoid doing these comparisons?

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/emitter.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:437: return member1.compareTo(member2);
Should the list already be sorted? After all, these elements are stored in the
ClassElement, and we should build that list sorted when creating the
ClassElement.

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:499: sortedClasses.sort((ClassElement
class1, ClassElement class2) {
I guess this one is hard to sort eagerly. How about assigning an index to the
ClassElement when it's created, so the order is only dependent on the way we
scan files?

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:587: member.position());
Maybe a FunctionElement should contain a list of closure elements?

[floitsch, 2012-05-08 15:26:44]

PTAL.

[kasperl, 2012-05-09 08:15:20]

LGTM. The output order is implicitly defined through the class ids and the way
the diet parser runs through stuff. Maybe adding a test so we don't regress the
ordering would be a good thing?

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
File lib/compiler/implementation/emitter.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:492: sortedClasses.sort((ClassElement
class1, ClassElement class2) {
Maybe describe what kind of sorting this implies? Here you are assuming that the
ids are increasing within a source file?

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
File lib/compiler/implementation/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
lib/compiler/implementation/ssa/closure.dart:19:
compiler.universe.getNextFreeClassId()) {
So all closures come after the classes in a source file? Maybe add a comment to
that extent.

[ngeoffray, 2012-05-09 08:58:27]

LGTM! Very nice and simple :)

[floitsch, 2012-05-09 10:55:07]

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/emitter.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:437: return member1.compareTo(member2);
On 2012/05/08 07:58:02, ngeoffray wrote:
> Should the list already be sorted? After all, these elements are stored in the
> ClassElement, and we should build that list sorted when creating the
> ClassElement.

Removed sorting of instance members.

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:499: sortedClasses.sort((ClassElement
class1, ClassElement class2) {
On 2012/05/08 07:58:02, ngeoffray wrote:
> I guess this one is hard to sort eagerly. How about assigning an index to the
> ClassElement when it's created, so the order is only dependent on the way we
> scan files?

Done.

https://chromiumcodereview.appspot.com/10310040/diff/4001/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:587: member.position());
On 2012/05/08 07:58:02, ngeoffray wrote:
> Maybe a FunctionElement should contain a list of closure elements?

change reverted.

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
File lib/compiler/implementation/emitter.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
lib/compiler/implementation/emitter.dart:492: sortedClasses.sort((ClassElement
class1, ClassElement class2) {
On 2012/05/09 08:15:20, kasperl wrote:
> Maybe describe what kind of sorting this implies? Here you are assuming that
the
> ids are increasing within a source file?

I added a comment. We don't rely on the sorting and it is not crucial that the
ids increase with the source file. But yes. that's the intention.

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
File lib/compiler/implementation/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/10310040/diff/2014/lib/compiler/implem...
lib/compiler/implementation/ssa/closure.dart:19:
compiler.universe.getNextFreeClassId()) {
On 2012/05/09 08:15:20, kasperl wrote:
> So all closures come after the classes in a source file? Maybe add a comment
to
> that extent.

Done.