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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11412086: Make 'where' lazy. (Closed)

Created:
8 years, 1 month ago by floitsch
Modified:
8 years ago
CC:
reviews_dartlang.org, Anders Johnsen
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Update some test names. #

Patch Set 3 : Rebase #

Total comments: 12

Patch Set 4 : FilteredIterable/Iterator -> WhereIterable/Iterator. #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+215 lines, -404 lines) Patch
M pkg/intl/test/date_time_format_test_core.dart View 2 2 chunks +2 lines, -2 lines 0 comments Download
M pkg/unittest/lib/html_enhanced_config.dart View 1 chunk +3 lines, -2 lines 0 comments Download
M pkg/unittest/lib/mock.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/unittest/lib/unittest.dart View 1 chunk +1 line, -1 line 0 comments Download
M runtime/lib/array.dart View 1 2 3 2 chunks +2 lines, -6 lines 4 comments Download
M runtime/lib/byte_array.dart View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M runtime/lib/expando_patch.dart View 1 chunk +1 line, -1 line 0 comments Download
M runtime/lib/growable_array.dart View 1 2 3 1 chunk +1 line, -3 lines 2 comments Download
M samples/clock/balls.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/swarm/DataSource.dart View 2 1 chunk +0 lines, -3 lines 0 comments Download
M samples/swarm/swarm_ui_lib/base/AnimationScheduler.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/swarm/swarm_ui_lib/layout/GridLayout.dart View 2 2 chunks +2 lines, -2 lines 0 comments Download
M samples/swarm/swarm_ui_lib/observable/observable.dart View 2 3 chunks +5 lines, -3 lines 0 comments Download
M samples/swarm/swarm_ui_lib/view/CompositeView.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/third_party/dromaeo/Suites.dart View 2 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/compile_time_constants.dart View 2 chunks +7 lines, -7 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/dart_backend/backend.dart View 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart View 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_backend/native_emitter.dart View 1 chunk +3 lines, -3 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/lib/interceptors.dart View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/tree/nodes.dart View 1 2 3 1 chunk +2 lines, -5 lines 0 comments Download
M sdk/lib/_internal/dartdoc/lib/dartdoc.dart View 1 chunk +1 line, -2 lines 0 comments Download
M sdk/lib/collection/collections.dart View 2 1 chunk +0 lines, -7 lines 0 comments Download
M sdk/lib/core/iterable.dart View 1 2 3 3 chunks +19 lines, -21 lines 0 comments Download
M sdk/lib/core/sequences.dart View 1 2 3 1 chunk +1 line, -8 lines 0 comments Download
M sdk/lib/html/dart2js/html_dart2js.dart View 2 43 chunks +45 lines, -104 lines 0 comments Download
M sdk/lib/html/dartium/html_dartium.dart View 42 chunks +43 lines, -102 lines 0 comments Download
M sdk/lib/html/scripts/idlparser.dart View 2 chunks +7 lines, -7 lines 0 comments Download
M sdk/lib/html/src/CssClassSet.dart View 2 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/html/src/FilteredElementList.dart View 2 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/html/src/_Collections.dart View 2 2 chunks +0 lines, -13 lines 0 comments Download
M sdk/lib/html/templates/html/dart2js/impl_SelectElement.darttemplate View 1 chunk +2 lines, -2 lines 0 comments Download
M sdk/lib/html/templates/html/impl/impl_Element.darttemplate View 1 2 3 2 chunks +4 lines, -16 lines 0 comments Download
M sdk/lib/html/templates/html/impl/impl_Node.darttemplate View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M sdk/lib/html/templates/html/impl/impl_NodeList.darttemplate View 1 2 3 3 chunks +3 lines, -5 lines 0 comments Download
M sdk/lib/html/templates/immutable_list_mixin.darttemplate View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M sdk/lib/svg/dart2js/svg_dart2js.dart View 1 2 3 9 chunks +9 lines, -18 lines 0 comments Download
M sdk/lib/svg/dartium/svg_dartium.dart View 1 2 3 9 chunks +9 lines, -18 lines 0 comments Download
M tests/corelib/list_test.dart View 2 1 chunk +1 line, -1 line 0 comments Download
M tests/corelib/queue_test.dart View 2 2 chunks +2 lines, -2 lines 0 comments Download
M tests/html/documentfragment_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/html/element_classes_test.dart View 2 1 chunk +3 lines, -3 lines 0 comments Download
M tests/html/element_test.dart View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M tests/html/node_test.dart View 1 2 chunks +6 lines, -5 lines 0 comments Download
M tests/html/queryall_test.dart View 1 1 chunk +2 lines, -2 lines 0 comments Download
M tests/html/xmldocument_test.dart View 2 1 chunk +3 lines, -3 lines 0 comments Download
M tests/html/xmlelement_test.dart View 2 1 chunk +3 lines, -3 lines 0 comments Download
M utils/tests/pub/test_pub.dart View 2 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
floitsch
8 years, 1 month ago (2012-11-20 01:22:03 UTC) #1
Anders Johnsen
LGTM, do you have any performance numbers for this change? https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart File runtime/lib/array.dart (left): https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart#oldcode77 ...
8 years, 1 month ago (2012-11-20 06:32:21 UTC) #2
Lasse Reichstein Nielsen
LGTM https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart File runtime/lib/array.dart (right): https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart#newcode76 runtime/lib/array.dart:76: Iterable<E> where(bool f(E element)) => new FilteredIterable<E>(this, f); ...
8 years, 1 month ago (2012-11-20 11:44:57 UTC) #3
Ivan Posva
DBC -ip https://codereview.chromium.org/11412086/diff/50/runtime/lib/array.dart File runtime/lib/array.dart (right): https://codereview.chromium.org/11412086/diff/50/runtime/lib/array.dart#newcode76 runtime/lib/array.dart:76: Iterable<E> where(bool f(E element)) => new WhereIterable<E>(this, ...
8 years ago (2012-11-26 18:30:40 UTC) #4
floitsch
8 years ago (2012-11-28 13:49:38 UTC) #5
Message was sent while issue was closed. 
Ivan's comments are addressed in https://codereview.chromium.org//11416240

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart
File runtime/lib/array.dart (left):

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart#old...
runtime/lib/array.dart:77: return Collections.where(this, new
_GrowableObjectArray<E>(), f);
On 2012/11/20 06:32:21, ajohnsen wrote:
> You could clean up Collections as you go on with this, to make sure we remove
> the functions.

I usually remove the functions there.

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/array.dart#new...
runtime/lib/array.dart:76: Iterable<E> where(bool f(E element)) => new
FilteredIterable<E>(this, f);
On 2012/11/20 11:44:57, Lasse Reichstein Nielsen wrote:
> FilteredIterable => WhereIterable

Done.

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/growable_array...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/11412086/diff/1049/runtime/lib/growable_array...
runtime/lib/growable_array.dart:187: Iterable<T> where(bool f(T element)) => new
FilteredIterable<T>(this, f);
On 2012/11/20 11:44:57, Lasse Reichstein Nielsen wrote:
> We can't just inherit this from Iterable?

Yes. we could, but currently this class is not extending any other class. I
didn't want to change this in this CL.

https://codereview.chromium.org/11412086/diff/1049/samples/swarm/swarm_ui_lib...
File samples/swarm/swarm_ui_lib/base/AnimationScheduler.dart (right):

https://codereview.chromium.org/11412086/diff/1049/samples/swarm/swarm_ui_lib...
samples/swarm/swarm_ui_lib/base/AnimationScheduler.dart:66: _callbacks =
_callbacks.where((CallbackData e) => e.id != id).toList();
On 2012/11/20 11:44:57, Lasse Reichstein Nielsen wrote:
> We want a mutating method on Collection: removeAllMatching.

Agreed. Not sure where, but it is a common task.

https://codereview.chromium.org/11412086/diff/1049/sdk/lib/core/iterable.dart
File sdk/lib/core/iterable.dart (right):

https://codereview.chromium.org/11412086/diff/1049/sdk/lib/core/iterable.dart...
sdk/lib/core/iterable.dart:38: * Returns a lazy [Queryable] with all elements
that satisfy the
On 2012/11/20 06:32:21, ajohnsen wrote:
> Queryable?

Done.

https://codereview.chromium.org/11412086/diff/1049/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11412086/diff/1049/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:22248: Iterable<Element> where(bool
f(Element element)) => _filtered.where(f);
On 2012/11/20 11:44:57, Lasse Reichstein Nielsen wrote:
> Would it be worth it to make it:
>  =>  _childNodes.where((e) => e is Element && f(e));
> That avoids an extra layer of iterators.

I don't think there is an extra layer of iterators. Is there?

https://codereview.chromium.org/11412086/diff/50/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11412086/diff/50/runtime/lib/array.dart#newco...
runtime/lib/array.dart:76: Iterable<E> where(bool f(E element)) => new
WhereIterable<E>(this, f);
On 2012/11/26 18:30:40, Ivan Posva wrote:
> ditto: curlys

Done.

https://codereview.chromium.org/11412086/diff/50/runtime/lib/array.dart#newco...
runtime/lib/array.dart:219: Iterable<E> where(bool f(E element)) => new
WhereIterable<E>(this, f);
On 2012/11/26 18:30:40, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/11412086/diff/50/runtime/lib/growable_array.dart
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/11412086/diff/50/runtime/lib/growable_array.d...
runtime/lib/growable_array.dart:187: Iterable<T> where(bool f(T element)) => new
WhereIterable<T>(this, f);
On 2012/11/26 18:30:40, Ivan Posva wrote:
> ditto

Done.

Powered by Google App Engine
This is Rietveld 408576698