Count external string resources that were assigned to symbol strings.

When we do native memory snapshot we counts only external string resources that have been added to heap->external_string_table_.
I found that not all the strings that have external resources were added to this table.
We do not add Symbols to the table probably because they have different lifetime.
But these symbols also could have external resources. As example it could happen when we assign innerHtml.
The simplest solution is to visit symbol_table too.

BUG=none
TEST=VisitExternalStrings
R=yurys

[yurys, 2012-10-26 09:23:39]

lgtm

[Yang, 2012-10-26 15:58:36]

On 2012/10/26 09:29:19, loislo wrote:

It seems to me that none of the strings made external using V8::MakeExternal is
added to the external string table, not only symbols. On the other hand, if an
existing external string is converted to a symbol, it would be counted twice
since it's both in the external string table and in the symbol table.
I think the fix should be adding external strings produced by V8::MakeExternal
to the external string table.

[loislo, 2012-10-26 17:42:55]

new version was uploaded

[yurys, 2012-10-29 06:24:57]

On 2012/10/26 17:42:55, loislo wrote:
> new version was uploaded

It would make sense to also add a test for the case that Yang mentioned when
existing external string is converted to a symbol.

[Yang, 2012-10-29 15:40:48]

On 2012/10/29 06:24:57, Yury Semikhatsky wrote:
> On 2012/10/26 17:42:55, loislo wrote:
> > new version was uploaded
> 
> It would make sense to also add a test for the case that Yang mentioned when
> existing external string is converted to a symbol.

I sensed some pitfalls in this issue, so I discussed it with Erik. Summary:

The reason we have an external string table (EST) is so that at a late phase (B)
of GC, we check which strings in the EST is alive, and call the dispose callback
on those that are not (and remove them from the EST).

In a prior phase (A) we also do that for strings in the symbol table: if we find
dead external strings in the symbol table, we call its dispose callback.

Consider an external string that has been turned into a symbol (case 1): it's
both in the EST and in the symbol table. When it dies, both phases (A) and (B)
stumble upon it and call its callback, but only A actually succeeds in calling
the callback, since it's then set to NULL so that (B) ignores it.

A symbol that has been externalized (case 2) is only dealt with in (A), since
it's not in the EST. The reason is that this way we can save space in the EST.

Visiting both the symbol table and the EST potentionally overvisits strings of
case 1.

The question is, is overvisiting a problem? If not, we can just do that. If yes,
we have some options:
1) not deal with external strings in (A) and always add case 2 strings to the
EST, which increases the required space in the EST.
2) remove case 1 strings from the EST, which is kind of expensive
3) visit strings in the EST first and ignore those that are symbols, since those
are in the symbol table, that we visit thereafter.

I strongly favor the third option. Sorry about the incorrect comment I gave
earlier!

[Yang, 2012-10-29 16:38:24]

On 2012/10/29 15:40:48, Yang wrote:
> On 2012/10/29 06:24:57, Yury Semikhatsky wrote:
> > On 2012/10/26 17:42:55, loislo wrote:
> > > new version was uploaded
> > 
> > It would make sense to also add a test for the case that Yang mentioned when
> > existing external string is converted to a symbol.
> 
> I sensed some pitfalls in this issue, so I discussed it with Erik. Summary:
> 
> The reason we have an external string table (EST) is so that at a late phase
(B)
> of GC, we check which strings in the EST is alive, and call the dispose
callback
> on those that are not (and remove them from the EST).
> 
> In a prior phase (A) we also do that for strings in the symbol table: if we
find
> dead external strings in the symbol table, we call its dispose callback.
> 
> Consider an external string that has been turned into a symbol (case 1): it's
> both in the EST and in the symbol table. When it dies, both phases (A) and (B)
> stumble upon it and call its callback, but only A actually succeeds in calling
> the callback, since it's then set to NULL so that (B) ignores it.
> 
> A symbol that has been externalized (case 2) is only dealt with in (A), since
> it's not in the EST. The reason is that this way we can save space in the EST.
> 
> Visiting both the symbol table and the EST potentionally overvisits strings of
> case 1.
> 
> The question is, is overvisiting a problem? If not, we can just do that. If
yes,
> we have some options:
> 1) not deal with external strings in (A) and always add case 2 strings to the
> EST, which increases the required space in the EST.
> 2) remove case 1 strings from the EST, which is kind of expensive
> 3) visit strings in the EST first and ignore those that are symbols, since
those
> are in the symbol table, that we visit thereafter.
> 
> I strongly favor the third option. Sorry about the incorrect comment I gave
> earlier!

I implemented option 3 in https://chromiumcodereview.appspot.com/11340010/

[Yang, 2012-11-02 12:47:25]

On 2012/10/29 16:38:24, Yang wrote:
> On 2012/10/29 15:40:48, Yang wrote:
> > On 2012/10/29 06:24:57, Yury Semikhatsky wrote:
> > > On 2012/10/26 17:42:55, loislo wrote:
> > > > new version was uploaded
> > > 
> > > It would make sense to also add a test for the case that Yang mentioned
when
> > > existing external string is converted to a symbol.
> > 
> > I sensed some pitfalls in this issue, so I discussed it with Erik. Summary:
> > 
> > The reason we have an external string table (EST) is so that at a late phase
> (B)
> > of GC, we check which strings in the EST is alive, and call the dispose
> callback
> > on those that are not (and remove them from the EST).
> > 
> > In a prior phase (A) we also do that for strings in the symbol table: if we
> find
> > dead external strings in the symbol table, we call its dispose callback.
> > 
> > Consider an external string that has been turned into a symbol (case 1):
it's
> > both in the EST and in the symbol table. When it dies, both phases (A) and
(B)
> > stumble upon it and call its callback, but only A actually succeeds in
calling
> > the callback, since it's then set to NULL so that (B) ignores it.
> > 
> > A symbol that has been externalized (case 2) is only dealt with in (A),
since
> > it's not in the EST. The reason is that this way we can save space in the
EST.
> > 
> > Visiting both the symbol table and the EST potentionally overvisits strings
of
> > case 1.
> > 
> > The question is, is overvisiting a problem? If not, we can just do that. If
> yes,
> > we have some options:
> > 1) not deal with external strings in (A) and always add case 2 strings to
the
> > EST, which increases the required space in the EST.
> > 2) remove case 1 strings from the EST, which is kind of expensive
> > 3) visit strings in the EST first and ignore those that are symbols, since
> those
> > are in the symbol table, that we visit thereafter.
> > 
> > I strongly favor the third option. Sorry about the incorrect comment I gave
> > earlier!
> 
> I implemented option 3 in https://chromiumcodereview.appspot.com/11340010/

Closing this since the issue has been solved in r12841.