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

feat(Scope): Instrument Scope to use User tags in Dart Obervatory #1138

Closed
wants to merge 1 commit into from

Conversation

mvuksano
Copy link
Contributor

No description provided.

@mvuksano mvuksano added cla: yes and removed cla: no labels Jun 10, 2014
@@ -667,6 +667,8 @@ class RootScope extends Scope {
* [ScopeDigestTTL].
*/
void digest() {
var digestTag = new UserTag('Digest');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you reuse a static/final tag instead of creating a new one each time ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to @vicb's comment

@mvuksano
Copy link
Contributor Author

@jbdeboer related to #1119, wouldn't all user code be run using scope.watch? If this is OK to assume we could expose a public version of scope.watch() which would set the correct tags when using. Internally we could use different scope.watch method which would set the correct flags for internal usage. Something like:

ScopeInternal extends Scope {
  Scope _delegate;
  ScopeInternal(this._delegate);

  ...

  runAsync(fn()) {
    var angularCodeTag = new UserTag('angular');
    var prevTag = angularCodeTag.makeCurrent();
    _delegate.runAsync(fn)
    prevTag.makeCurrent();
  }
  ...
}

Not sure if this is the right approach.. I'm still thinking this through... Any suggestions are welcome :)

final UserTag _runAsyncTag = new UserTag('RunAsync');
final UserTag _domReadTag = new UserTag('DomRead');
final UserTag _domWriteTag = new UserTag('DomWrite');
UserTag _previousTag;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved locally to the functions

@mvuksano
Copy link
Contributor Author

@mhevery & @jbdeboer This now includes quite a bit of information about scope. Any other ideas what might be useful to instrument around here?

Here's a screenshot to see what current output might look like.

screen shot 2014-06-11 at 11 47 51 pm

@mvuksano
Copy link
Contributor Author

Btw, it's an interesting insight that reaction function during digest is by far the most expensive in this example.

AvgStopwatch processStopwatch,
String state,
num sequenceNumber}) {
UserTag detectChangesTag = new UserTag('DetectChanges / $state${sequenceNumber != null ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserTags need to be stored in global variable. We can't be making new ones ever time this code runs.

@mhevery
Copy link
Contributor

mhevery commented Jun 13, 2014

Nice work. Just make sure that we don't constantly create new tags.

@mhevery
Copy link
Contributor

mhevery commented Jun 13, 2014

LGTM, Otherwise

@@ -367,6 +368,8 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
class RootWatchGroup extends WatchGroup {
final FieldGetterFactory _fieldGetterFactory;
Watch _dirtyWatchHead, _dirtyWatchTail;
Map<String, UserTag> _detectChangesTags = new Map();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use List instead of Map. Initialize List size from ScopeTTL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mhevery
Copy link
Contributor

mhevery commented Jun 16, 2014

Also instrument View creation. This can be done in ViewFactory

https://github.com/angular/angular.dart/blob/master/lib/core_dom/tagging_view_factory.dart#L21

@mvuksano mvuksano closed this in c21ac7e Jun 18, 2014
@jbdeboer
Copy link
Contributor

@markovuksanovic @tbosch How do we know this change doesn't cause a performance regression in production mode (when tags are not needed?)

@mvuksano
Copy link
Contributor Author

@jbdeboer I guess the only way to answer your question is to run some benchmarks. Do you know if there are some benchmarks that are already in place which could be used for this? I've seen some benchmarks added recently.

@mhevery
Copy link
Contributor

mhevery commented Jun 24, 2014

The tests I have run do not show any impact on performance. Changing tags
is very cheap, about the cost of a field read/write, and they are not
placed in critical path of cade.

On Tue, Jun 24, 2014 at 6:59 AM, Marko Vuksanovic [email protected]
wrote:

@jbdeboer https://github.com/jbdeboer I guess the only way to answer
your question is to run some benchmarks. Do you know if there are some
benchmarks that are already in place which could be used for this? I've
seen some benchmarks added recently.


Reply to this email directly or view it on GitHub
#1138 (comment).

dsalsbury pushed a commit to dsalsbury/angular.dart that referenced this pull request Jul 16, 2014
dsalsbury pushed a commit to dsalsbury/angular.dart that referenced this pull request Jul 16, 2014
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.

4 participants