This repository was archived by the owner on Apr 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Alow disabling debug info such as ng-scope, ng-binding classes #8802
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The compiler and ngBind directives add binding information (`ng-binding` CSS class and `$binding` data property) to elements when they are bound to the scope. This is only to aid testing and debugging for tools such as Protractor and Batarang. In production this is unnecessary and add a performance penalty. This can be now disabled by calling `$compileProvider.debugInfoEnabled(false)` in a module `config` block: ``` someModule.config(['$compileProvider', function($compileProvider) { $compileProvider.debugInfoEnabled(false); }]); ``` In the bench/apps/largetable-bp benchmark this change, with debug info disabled, improved by ~140ms, that is 10%. Measuring the "create" phase, 25 loops, mean time ~1340ms -> ~1200ms. We were storing the whole `interpolationFn` in the `$binding` data on elements but this function was bringing a lot of closure variables with it and so was consuming unwanted amounts of memory. Now we are only storing the parsed interpolation expressions from the binding (i.e. the values of `interpolationFn.expressions`). BREAKING CHANGE: The value of `$binding` data property on an element is always an array now and the expressions do not include the curly braces `{{ ... }}`.
The compiler adds scope information (`ng-scope` CSS class and `$scope` data property) to elements when the are bound to the scope. This is mostly to aid debugging tools such as Batarang. In production this should be unnecesary and adds a performance penalty. In the bench/apps/largetable-bp this change caused an improvement of ~100ms (7%). This can be now disabled by calling `$compileProvider.debugInfoEnabled(false)` in a module `config` block: ``` someModule.config(['$compileProvider', function($compileProvider) { $compileProvider.debugInfoEnabled(false); }]); ``` In the bench/apps/largetable-bp benchmark this change, with debug info disabled, improved by ~120ms, that is ~10%. Measuring the "create" phase, 25 loops, mean time ~1200ms -> ~1080ms.
After upgrading, Protractor requires exact string that is used in the binding.
…array Instead of knowing about `.expressions` property, it just accepts a single expression or an array of expressions.
We run unit tests in “strict” mode and thus can’t monkey-patch `window.location` nor `window.location.reload`. In order to avoid full page reload, we could pass location as argument, or another level of indirection, something like this: ```js var ourGlobalFunkyLocation = window.location; function reloadWithDebugInfo() { window.name = 'NG_ENABLE_DEBUG_INFO!' + window.name; ourGlobalFunkyLocation.reload(); } // in the test ourGlobalFunkyLocation = { reload: function() {} }; reloadWithDebugInfo(); ourGlobalFunkyLocation = window.location; ``` I don’t think any of these make sense, just so that we can test setting `window.name`. If the `reloadWithDebugInfo` function was more complicated, I would do it. I don’t think it’s worthy to confuse production code with extra logic which purpose was only to make testing possible.
`$$addScopeInfo` used to accept either DOM Node or jqLite/jQuery wrapper. This commit simplifies the method to always require jqLite/jQuery wrapper and thus remove the `element.data` condition which was wrong. If `element` was a raw comment element, the `data` property was a string (the value of the comment) and an exception was thrown.
To follow our convention (at least in this file): if it’s a jqLite/jQuery wrapper than the variable name starts with `$`.
In a93f03d and d37f103 we changed the compiler and ngBind to add debugging CSS classes (i.e. ng-scope, ng-binding) in linking function. This simplified the code and made sense under the original assumptions that the debug info will be disabled by default. That is however not the case - debug info is enabled by default. When debug info is enabled, this change improves the largetable-bp benchmark by ~580ms, that is 30% faster. Measuring the “create” phase, 25 loops, meantime ~1920ms -> ~1340ms. This change does not affect performance when debug info is disabled.
For easier debugging.
Merged as c6bde52 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is based on Pete's PR.