External references should not affect dominance relation.

External references should not affect dominance relation.

Separate objects into two groups: reachable from a window (user),
and unreachable (system). Then do not take into account links
that come from system group to the user group when calculating
dominance relation.

Committed: https://code.google.com/p/v8/source/detail?r=11335

[alexeif, 2012-04-13 15:58:01]

Misha, I have to update the test because it was failing in one of the
configurations. Could you please take a look once more at the CL.

[mnaganov (inactive), 2012-04-16 08:40:25]

https://chromiumcodereview.appspot.com/10086006/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/1/src/profile-generator....
src/profile-generator.cc:3303: if (children[i].type() ==
HeapGraphEdge::kShortcut) {
For the sake of clarity, I'd rename 'SetRootShortcutReference' to
'SetWindowReference' and add 'IsWindowReference' to filter out window objects.
Otherwise, this construction is fragile.

https://chromiumcodereview.appspot.com/10086006/diff/1/test/cctest/test-heap-...
File test/cctest/test-heap-profiler.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/1/test/cctest/test-heap-...
test/cctest/test-heap-profiler.cc:156: printf("ALF: %d %d %d %d\n",
x->GetSelfSize(), x1->GetSelfSize(), x2->GetSelfSize(), x->GetRetainedSize());
Do you need this printf? Also, please mind the line length.

[alexeif, 2012-04-16 10:49:11]

https://chromiumcodereview.appspot.com/10086006/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/1/src/profile-generator....
src/profile-generator.cc:3303: if (children[i].type() ==
HeapGraphEdge::kShortcut) {
On 2012/04/16 08:40:26, Mikhail Naganov (Chromium) wrote:
> For the sake of clarity, I'd rename 'SetRootShortcutReference' to
> 'SetWindowReference' and add 'IsWindowReference' to filter out window objects.
> Otherwise, this construction is fragile.

Done.

https://chromiumcodereview.appspot.com/10086006/diff/1/test/cctest/test-heap-...
File test/cctest/test-heap-profiler.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/1/test/cctest/test-heap-...
test/cctest/test-heap-profiler.cc:156: printf("ALF: %d %d %d %d\n",
x->GetSelfSize(), x1->GetSelfSize(), x2->GetSelfSize(), x->GetRetainedSize());
On 2012/04/16 08:40:26, Mikhail Naganov (Chromium) wrote:
> Do you need this printf? Also, please mind the line length.
oops. fixed.

[mnaganov (inactive), 2012-04-16 12:16:54]

lgtm

[yurys, 2012-04-16 13:23:37]

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.cc:3339: void HeapSnapshotGenerator::FillPostorderIndexes(
I think it is still reverse postorder, isn't it?

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.h:566: bool reachable_from_window() { return
reachable_from_window_; }
Note, that there is no such think as Window in case of dedicated/shared web
workers. We may want a better name for the user root object.

[alexeif, 2012-04-16 13:37:23]

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.cc:3339: void HeapSnapshotGenerator::FillPostorderIndexes(
On 2012/04/16 13:23:37, Yury Semikhatsky wrote:
> I think it is still reverse postorder, isn't it?
Why reverse? I think it's direct postorder.

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.h (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.h:566: bool reachable_from_window() { return
reachable_from_window_; }
On 2012/04/16 13:23:37, Yury Semikhatsky wrote:
> Note, that there is no such think as Window in case of dedicated/shared web
> workers. We may want a better name for the user root object.
user_root?

[yurys, 2012-04-16 13:41:01]

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.cc:3339: void HeapSnapshotGenerator::FillPostorderIndexes(
On 2012/04/16 13:37:23, alexeif wrote:
> On 2012/04/16 13:23:37, Yury Semikhatsky wrote:
> > I think it is still reverse postorder, isn't it?
> Why reverse? I think it's direct postorder.

From http://en.wikipedia.org/wiki/Tree_traversal#Postorder_Traversal:
void postorder(tree t) 
{
        if(t == NULL)
                return;
        postorder(t->left);
        postorder(t->right);
        printf("%d ", t->val);
}

In our case we'd have:
void reverse_postorder(tree t) 
{
        if(t == NULL)
                return;
        postorder(t->right);
        postorder(t->left);
        printf("%d ", t->val);
}

[alexeif, 2012-04-16 13:45:00]

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
File src/profile-generator.cc (right):

https://chromiumcodereview.appspot.com/10086006/diff/6001/src/profile-generat...
src/profile-generator.cc:3339: void HeapSnapshotGenerator::FillPostorderIndexes(
On 2012/04/16 13:41:01, Yury Semikhatsky wrote:
> On 2012/04/16 13:37:23, alexeif wrote:
> > On 2012/04/16 13:23:37, Yury Semikhatsky wrote:
> > > I think it is still reverse postorder, isn't it?
> > Why reverse? I think it's direct postorder.
> 
> From http://en.wikipedia.org/wiki/Tree_traversal#Postorder_Traversal:
> void postorder(tree t) 
> {
>         if(t == NULL)
>                 return;
>         postorder(t->left);
>         postorder(t->right);
>         printf("%d ", t->val);
> }
> 
> In our case we'd have:
> void reverse_postorder(tree t) 
> {
>         if(t == NULL)
>                 return;
>         postorder(t->right);
>         postorder(t->left);
>         printf("%d ", t->val);
> }
The children is not ordered in our case. So speaking of an order regarding
children is meaningless.