Skip to content

perf(DirectiveDependency): iterate only once over Dependency properties#918

Closed
vicb wants to merge 1 commit into
angular:masterfrom
vicb:0311-elInj
Closed

perf(DirectiveDependency): iterate only once over Dependency properties#918
vicb wants to merge 1 commit into
angular:masterfrom
vicb:0311-elInj

Conversation

@vicb

@vicb vicb commented Mar 11, 2015

Copy link
Copy Markdown
Contributor

The former code iterates 4 times over the list

@mhevery mhevery added @lgtm action: merge The PR is ready for merge by the caretaker labels Mar 13, 2015
@vsavkin

vsavkin commented Mar 13, 2015

Copy link
Copy Markdown
Contributor

@vicb could you provide a benchmark showing the perf gain? This happens during compilation so I double it makes a difference. But merging the loops imo makes code harder to reason about.

I don't think we should make speculative perf changes. Everything perf related should require some benchmark data showing that it makes a difference.

@vicb vicb closed this in c6893ac Mar 18, 2015
@vicb vicb removed the in progress label Mar 18, 2015
@vicb vicb deleted the 0311-elInj branch April 7, 2015 07:18
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants