Roll ICU 239289:258359 and add support for external ICU data tables

Roll ICU 239289:258359 and add support for external ICU data tables

The only binary that supports ICU data tables is d8. The location of the
data table file has to be passed via a command line switch:

  $ out/x64.optdebug/d8 --icu-data-file=out/x64.optdebug/icudtl.dat

BUG=72633, v8:3142
R=danno@chromium.org
LOG=y

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

[jochen (gone - plz use gerrit), 2014-03-25 08:39:57]

please review

the default for standalone builds is not to use an external data file.

[danno, 2014-03-25 08:47:21]

lgtm with comments

https://codereview.chromium.org/209323014/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/209323014/diff/1/include/v8.h#newcode4959
include/v8.h:4959: * of the data file as to be passed.
has to be provided

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
File src/icu_util.cc (right):

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode82
src/icu_util.cc:82: delete[] addr;
Not sure if we care about deleting this on V8 cleanup, valgrind might complain.

[jochen (gone - plz use gerrit), 2014-03-25 12:04:11]

https://codereview.chromium.org/209323014/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/209323014/diff/1/include/v8.h#newcode4959
include/v8.h:4959: * of the data file as to be passed.
On 2014/03/25 08:47:21, danno wrote:
> has to be provided

Done.

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
File src/icu_util.cc (right):

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode82
src/icu_util.cc:82: delete[] addr;
On 2014/03/25 08:47:21, danno wrote:
> Not sure if we care about deleting this on V8 cleanup, valgrind might
complain.

I don't think we run d8 when build in chromium under valgrind. And if yes, we
need to add a suppression to valgrind.

[jochen (gone - plz use gerrit), 2014-03-25 12:05:43]

Committed patchset #1 manually as r20239 (presubmit successful).

[Sven Panne, 2014-03-25 12:10:56]

Again a bit late, but...

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
File src/icu_util.cc (right):

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode82
src/icu_util.cc:82: delete[] addr;
On 2014/03/25 12:04:11, jochen wrote:
> On 2014/03/25 08:47:21, danno wrote:
> > Not sure if we care about deleting this on V8 cleanup, valgrind might
> complain.
> 
> I don't think we run d8 when build in chromium under valgrind. And if yes, we
> need to add a suppression to valgrind.

Hmmm, d8 should really be valgrind-clean, and it was (at least at some points in
the past ;-). If this CL breaks this, we should add some kind of cleanup call
like DeinitializeICU().

[jungshik at Google, 2014-03-26 05:53:14]

This is also late, but I'm just wondering about mmap.

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
File src/icu_util.cc (right):

https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode81
src/icu_util.cc:81: if (fread(addr, 1, size, inf) != size) {
Would it be a good idea to use memory map (I saw some utilities functions in v8
tree for mmap) instead?

[jochen (gone - plz use gerrit), 2014-03-26 08:21:35]

On 2014/03/26 05:53:14, Jungshik Shin wrote:
> This is also late, but I'm just wondering about mmap.
> 
> https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
> File src/icu_util.cc (right):
> 
> https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode81
> src/icu_util.cc:81: if (fread(addr, 1, size, inf) != size) {
> Would it be a good idea to use memory map (I saw some utilities functions in
v8
> tree for mmap) instead?

there's just support for mmaping anonymous memory.

also, this code is only used for testing, so I don't think it needs to be too
fancy

[jungshik at Google, 2014-03-27 00:03:13]

On 2014/03/26 08:21:35, jochen wrote:
> On 2014/03/26 05:53:14, Jungshik Shin wrote:
> > This is also late, but I'm just wondering about mmap.
> > 
> > https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc
> > File src/icu_util.cc (right):
> > 
> > https://codereview.chromium.org/209323014/diff/1/src/icu_util.cc#newcode81
> > src/icu_util.cc:81: if (fread(addr, 1, size, inf) != size) {
> > Would it be a good idea to use memory map (I saw some utilities functions in
> v8
> > tree for mmap) instead?
> 
> there's just support for mmaping anonymous memory.
> 
> also, this code is only used for testing, so I don't think it needs to be too
> fancy

Thanks for the reply. BTW, do we have to care about sample/shell.cc ?