Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

perf(dccd): Drop useless checks #1256

Closed
wants to merge 1 commit into from
Closed

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Jul 23, 2014

in _recordAdd(), previous could never be null as the list is never empty (it has at least a marker).
in _recordRemove(), when the list is composed of a single record, it must be the record we're trying to remove (and we assert that).

The perf gain should be marginal and is not reliably measure with the wg benchmark (hidden by the noise)

in _recordAdd(), previous could never be null as the list is never empty
(it has at least a marker).
in _recordRemove(), when the list is composed of a single record, it
must be the record we're trying to remove (and we assert that).
@vicb vicb added cla: yes and removed cla: no labels Jul 23, 2014
@jbdeboer
Copy link
Contributor

LGTM

@vicb vicb closed this in 209d14f Jul 31, 2014
@vicb vicb deleted the 0723-dccdPerf branch July 31, 2014 08:33
vicb added a commit that referenced this pull request Aug 5, 2014
in _recordAdd(), previous could never be null as the list is never empty
(it has at least a marker).
in _recordRemove(), when the list is composed of a single record, it
must be the record we're trying to remove (and we assert that).

Closes #1256
dsalsbury pushed a commit to dsalsbury/angular.dart that referenced this pull request Aug 7, 2014
in _recordAdd(), previous could never be null as the list is never empty
(it has at least a marker).
in _recordRemove(), when the list is composed of a single record, it
must be the record we're trying to remove (and we assert that).

Closes dart-archive#1256
@chirayuk
Copy link
Contributor

chirayuk commented Aug 9, 2014

This change does not work and breaks some apps. It has no real benefit. I'm reverting it.

@vicb
Copy link
Contributor Author

vicb commented Aug 9, 2014

@chirayuk Do you have more info on how the apps break ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants