[v8-dev] Fix command line parsing. (issue 452093002 by vogelh...@chromium.org)

2014-08-08 Thread vogelheim
Reviewers: Hannes Payer, Description: Fix command line parsing. For --min-parse-length --cache=code, SetFlagsFromCommandLine sees an argv like this: [.../blabla/d8, min-parse-length, NULL] It then calls strtol(NULL, ...), which strtol dislikes. It turns out that: - V8::Shell::SetOptions will

[v8-dev] Re: Fix command line parsing. (issue 452093002 by vogelh...@chromium.org)

2014-08-11 Thread vogelheim
Committed as r23005. (I inadvertently committed w/ 'git svn dcommit' rather than 'git cl dcommit', hence no automated message.) https://codereview.chromium.org/452093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message

[v8-dev] Re: Fix borked tests after r23354. (issue 505753002 by mstarzin...@chromium.org)

2014-08-25 Thread vogelheim
lgtm LGTM https://codereview.chromium.org/505753002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails

[v8-dev] Re: Correctly return value. Fix for r23355. (issue 500993002 by yang...@chromium.org)

2014-08-25 Thread vogelheim
lgtm https://codereview.chromium.org/500993002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Use correct BitField arguments in CEntryStub. (issue 494543003 by yang...@chromium.org)

2014-08-25 Thread vogelheim
lgtm https://codereview.chromium.org/494543003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Fix GN build after r23364. (issue 481333004 by mstarzin...@chromium.org)

2014-08-25 Thread vogelheim
lgtm https://codereview.chromium.org/481333004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Fix memory leak. Make V8 Linux - memcheck build bot happy. (issue 505103003 by vogelh...@chromium.org)

2014-08-26 Thread vogelheim
@machenbach: Can I somehow trigger the 'memcheck' buildbot to verify this actually resolves the issue? https://codereview.chromium.org/505103003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to

[v8-dev] Re: Fix memory leak. Make V8 Linux - memcheck build bot happy. (issue 505103003 by vogelh...@chromium.org)

2014-08-26 Thread vogelheim
Committed patchset #1 manually as 23395 (presubmit successful). https://codereview.chromium.org/505103003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Fix external snapshot serialization by null-terminating strings (issue 496253003 by ba...@chromium.org)

2014-08-26 Thread vogelheim
Sorry for the late answer; I overlooked this. Please feel free to ping me if you don't get an answer within 1 working day. https://codereview.chromium.org/496253003/diff/20001/tools/js2c.py File tools/js2c.py (right):

[v8-dev] Re: Fix external snapshot serialization by null-terminating strings (issue 496253003 by ba...@chromium.org)

2014-08-26 Thread vogelheim
lgtm Thanks for fixing this! https://codereview.chromium.org/496253003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop

[v8-dev] Re: MIPS: Move register conventions out of the IC classes. (issue 508673002 by balazs.kilv...@imgtec.com)

2014-08-26 Thread vogelheim
lgtm (I don't have much to say on the substance on the change, though. gyp+GN looks fine.) https://codereview.chromium.org/508673002/diff/40001/src/ic/mips/ic-conventions-mips.cc File src/ic/mips/ic-conventions-mips.cc (right):

[v8-dev] Partial revert of r23415. Original message: (issue 486543004 by vogelh...@chromium.org)

2014-08-26 Thread vogelheim
Reviewers: , Message: Committed patchset #2 manually as 23421 (tree was closed). Description: Partial revert of r23415. Original message: Enable more tests that no longer fail with TF. It looks like V8 Win32 - 1 still has issues. Try to revert only the affected test. BUG=

[v8-dev] Re: Properly separate host and target builds when using external natives. (issue 506983002 by ba...@chromium.org)

2014-08-27 Thread vogelheim
lgtm Thanks for fixing this! https://codereview.chromium.org/506983002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop

[v8-dev] Re: Fix versus for stupid C++ compilers. (issue 489733004 by tit...@chromium.org)

2014-08-27 Thread vogelheim
lgtm Thanks for fast response! https://codereview.chromium.org/489733004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and

[v8-dev] Re: Version 3.29.20.1 (merged r23454) (issue 484373003 by machenb...@chromium.org)

2014-08-27 Thread vogelheim
lgtm https://codereview.chromium.org/484373003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Remove C++11-ism, until all bots support it. (issue 509963002 by vogelh...@chromium.org)

2014-08-27 Thread vogelheim
Reviewers: Michael Achenbach, Message: Committed patchset #1 manually as 23463 (tree was closed). Description: Remove C++11-ism, until all bots support it. R=machenb...@chromium.org BUG= Committed: https://code.google.com/p/v8/source/detail?r=23463 Please review this at

[v8-dev] Re: Disable running some change tests on unsupported backends. (issue 515583002 by tit...@chromium.org)

2014-08-27 Thread vogelheim
lgtm https://codereview.chromium.org/515583002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Fix Windows failure after r23492. (issue 513223002 by mstarzin...@chromium.org)

2014-08-28 Thread vogelheim
lgtm Thanks for the quick fix! https://codereview.chromium.org/513223002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and

[v8-dev] Re: Fix test failures after r23492. (issue 517673002 by mstarzin...@chromium.org)

2014-08-28 Thread vogelheim
lgtm Oh, these modernist non-Posix extensions... https://codereview.chromium.org/517673002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from

[v8-dev] Re: Skip failing test on ARM after r23492. (issue 515753006 by mstarzin...@chromium.org)

2014-08-28 Thread vogelheim
lgtm https://codereview.chromium.org/515753006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Revert Move base library definitions from v8.gyp to base.gyp. (issue 517303002 by bmeu...@chromium.org)

2014-08-29 Thread vogelheim
lgtm Thanks! https://codereview.chromium.org/517303002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails

[v8-dev] Re: Properly separate host and target builds when using external natives. (issue 506983002 by ba...@chromium.org)

2014-08-29 Thread vogelheim
https://codereview.chromium.org/506983002/diff/20001/tools/gyp/v8.gyp File tools/gyp/v8.gyp (right): https://codereview.chromium.org/506983002/diff/20001/tools/gyp/v8.gyp#newcode281 tools/gyp/v8.gyp:281: '(PRODUCT_DIR)/snapshot_blob_host.bin', I'm a bit skeptical here: snapshot.cc and

[v8-dev] Re: Properly separate host and target builds when using external natives. (issue 506983002 by ba...@chromium.org)

2014-08-29 Thread vogelheim
lgtm https://codereview.chromium.org/506983002/diff/20001/tools/gyp/v8.gyp File tools/gyp/v8.gyp (right): https://codereview.chromium.org/506983002/diff/20001/tools/gyp/v8.gyp#newcode281 tools/gyp/v8.gyp:281: '(PRODUCT_DIR)/snapshot_blob_host.bin', Ah, yes, that makes sense. Thanks for the

[v8-dev] Re: MIPS: Refactoring InterfaceDescriptors away from code-stubs.h - external. (issue 516253002 by balazs.kilv...@imgtec.com)

2014-08-29 Thread vogelheim
lgtm I'll take it you'll submit this right after the change which introduces the corresponding sources? https://codereview.chromium.org/516253002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed

[v8-dev] Re: Fix missing bracket in gyp file from r23522. (issue 521613003 by rmcil...@chromium.org)

2014-08-29 Thread vogelheim
lgtm https://codereview.chromium.org/521613003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Add histogram timers for (de-)serialization during compilation. (issue 578263004 by vogelh...@chromium.org)

2014-09-19 Thread vogelheim
Reviewers: Yang, Description: Add histogram timers for (de-)serialization during compilation. R=yang...@chromium.org BUG= Please review this at https://codereview.chromium.org/578263004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+8, -1 lines): M

[v8-dev] Re: Add histogram timers for (de-)serialization during compilation. (issue 578263004 by vogelh...@chromium.org)

2014-09-22 Thread vogelheim
Committed patchset #1 (id:1) manually as 24127 (presubmit successful). https://codereview.chromium.org/578263004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group.

[v8-dev] Implement PersistentValueMap, a map that stores UniquePersistent values. (issue 189463019)

2014-03-10 Thread vogelheim
Reviewers: dcarney, Description: Implement PersistentValueMap, a map that stores UniquePersistent values. This is preparatory work to get rid of UnsafePersistent in blink. Related blink changes are here: https://codereview.chromium.org/180363004/ This patch is largely based on

[v8-dev] Re: Implement PersistentValueMap, a map that stores UniquePersistent values. (issue 189463019)

2014-03-10 Thread vogelheim
PTAL. https://codereview.chromium.org/189463019/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/189463019/diff/1/include/v8.h#newcode840 include/v8.h:840: V8_INLINE bool SetReturnValueFrom(const K key, On 2014/03/10 11:35:23, dcarney wrote: just SetReturnValue

[v8-dev] Re: Implement PersistentValueMap, a map that stores UniquePersistent values. (issue 189463019)

2014-03-11 Thread vogelheim
: UniquePersistent should always be passed by value. not ref Done. https://codereview.chromium.org/189463019/diff/20001/include/v8.h#newcode853 include/v8.h:853: UniquePersistentV Set(const K key, UniquePersistentV value) { On 2014/03/11 07:44:46, dcarney wrote: On 2014/03/10 19:00:46, vogelheim wrote

[v8-dev] Re: Implement PersistentValueMap, a map that stores UniquePersistent values. (issue 189463019)

2014-03-11 Thread vogelheim
https://codereview.chromium.org/189463019/diff/40001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/189463019/diff/40001/include/v8.h#newcode819 include/v8.h:819: templateclass K, class V, class Traits On 2014/03/11 15:12:05, dcarney wrote: need to add some small

[v8-dev] Implement PersistentValueMap, a map that stores UniquePersistent values. Part 2. (issue 197263002)

2014-03-12 Thread vogelheim
Reviewers: dcarney, Message: PTAL. 2nd try for PersistentValueMap, w/ fix for user-after-free discovered by Win32/debug bot. Description: Implement PersistentValueMap, a map that stores UniquePersistent values. This is preparatory work to get rid of UnsafePersistent in blink. The previous

[v8-dev] Move PersitentValueMap into seperate header, to avoid excessive polution (issue 195793024)

2014-03-14 Thread vogelheim
Reviewers: dcarney, Description: Move PersitentValueMap into seperate header, to avoid excessive polution of v8.h. R=dcar...@chromium.org BUG= Please review this at https://codereview.chromium.org/195793024/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+221,

[v8-dev] First attempt at providing default traits for PersistentValueMap. (issue 201643003)

2014-03-17 Thread vogelheim
Reviewers: dcarney, Message: This is a continuation of issue 195793024. Please have a look. I'm not really sure where to go with this, so the two patch sets are essentially two different proposals of where to go. The 2nd one looks nicer and is arguably easier to use, at least when you get

[v8-dev] Re: First attempt at providing default traits for PersistentValueMap. (issue 201643003)

2014-03-17 Thread vogelheim
PTAL. https://codereview.chromium.org/201643003/diff/60001/include/v8-util.h File include/v8-util.h (right): https://codereview.chromium.org/201643003/diff/60001/include/v8-util.h#newcode46 include/v8-util.h:46: namespace persistent_value_map_traits { On 2014/03/17 12:53:55, dcarney wrote: no

[v8-dev] Re: First attempt at providing default traits for PersistentValueMap. (issue 201643003)

2014-03-18 Thread vogelheim
It turns out I was being daft, and removing the default implementations works as it should. :-| I guess the remaining question is whether we stay with one Traits class, or try to some split this up? https://codereview.chromium.org/201643003/ -- -- v8-dev mailing list

[v8-dev] Re: First attempt at providing default traits for PersistentValueMap. (issue 201643003)

2014-03-18 Thread vogelheim
PTAL. I think this should address the review comments. https://codereview.chromium.org/201643003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe

[v8-dev] Re: First attempt at providing default traits for PersistentValueMap. (issue 201643003)

2014-03-18 Thread vogelheim
https://codereview.chromium.org/201643003/diff/120001/include/v8-util.h File include/v8-util.h (right): https://codereview.chromium.org/201643003/diff/120001/include/v8-util.h#newcode87 include/v8-util.h:87: static void Dispose(v8::Isolate* isolate, v8::UniquePersistentV value, On 2014/03/18

[v8-dev] Provide default traits for PersistentValueMap (issue 204343006)

2014-03-19 Thread vogelheim
Reviewers: dcarney, Message: Re-trying https://codereview.chromium.org/201643003 So... I tried several variants to fix this, but without any real success. This version - which provides empty default implementations - seems to work fine. At least it does on the trybots. I tried various

[v8-dev] Re: Revert FastElementsAccessor::SetLengthWithoutNormalize() handlified. (issue 208313015)

2014-03-24 Thread vogelheim
lgtm https://codereview.chromium.org/208313015/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Amend PersistentValueMap. (issue 212893007)

2014-03-26 Thread vogelheim
Reviewers: dcarney, Message: Please have a look. I'll update the dependent CLs shortly. Description: Amend PersistentValueMap: - Use the surrounding map (instead of Traits::Impl) for weak callback. - Provide for a fast reference to a mapped value. - Restructure Traits to accomondate for the

[v8-dev] Re: Amend PersistentValueMap. (issue 212893007)

2014-03-27 Thread vogelheim
https://codereview.chromium.org/212893007/diff/20001/include/v8-util.h File include/v8-util.h (right): https://codereview.chromium.org/212893007/diff/20001/include/v8-util.h#newcode256 include/v8-util.h:256: explicit PersistentValueReference(PersistentContainerValue value) On 2014/03/27

[v8-dev] Re: Amend PersistentValueMap. (issue 212893007)

2014-03-27 Thread vogelheim
On 2014/03/27 07:55:03, dcarney wrote: i realize that everywhere we can kIsWeak, we will have a problem when we introduce phantom references. We should introduce an enum somewhere with 2 values, kNotWeak and kWeak and use that everywhere instead of a boolean Done. If we want to help

[v8-dev] Re: Amend PersistentValueMap. (issue 212893007)

2014-03-27 Thread vogelheim
https://codereview.chromium.org/212893007/diff/40001/include/v8-util.h File include/v8-util.h (right): https://codereview.chromium.org/212893007/diff/40001/include/v8-util.h#newcode45 include/v8-util.h:45: enum PersistentValueMapCallbackType { On 2014/03/27 16:36:11, dcarney wrote: should be

[v8-dev] Implement PersistentValueVector, analogous to PersistentValueMap. (issue 216973002)

2014-03-28 Thread vogelheim
Reviewers: dcarney, Message: First implementation of PersistentValueVector. Q: This vector is rather simple and provides only the functionality that the current [vV]ectorUnsafePersistent need. In particular it doesn't bother with weak references or iterators, find, etc., since all the current

[v8-dev] Re: Remove V8_INLINE from v8-util.h. (issue 212693006)

2014-04-02 Thread vogelheim
Reviewers: jochen, dcarney, Message: Committed patchset #1 manually as r20432 (presubmit successful). Description: Remove V8_INLINE from v8-util.h. (These have been causing compilation problems on some platforms. For VS, V8_INLINE turns into 'force inline', which will then cause a problem when

[v8-dev] Add support for --raw and --omit to js2c. (issue 225723002)

2014-04-04 Thread vogelheim
Reviewers: jochen, Message: Not sure whether I'm on the right track; please have a look at this. Description: Add support for --raw and --omit to js2c. --raw writes the raw source data to a separate file --omit allows omitting the source data from the generated files. The intention is

[v8-dev] Remove V8_ALLOW_ACCESS_TO_RAW_HANDLE_CONSTRUCTOR. (issue 230443004)

2014-04-09 Thread vogelheim
Reviewers: dcarney, Message: Minor cleanup. I won't be able to check-in until the usages in Chromium have been removed, though. (That CL has been sent, too.) Description: Remove V8_ALLOW_ACCESS_TO_RAW_HANDLE_CONSTRUCTOR. The usage of this define has been obsoleted by removal of

[v8-dev] Re: Remove V8_ALLOW_ACCESS_TO_RAW_HANDLE_CONSTRUCTOR. (issue 230443004)

2014-04-10 Thread vogelheim
Committed patchset #1 manually as r20656 (presubmit successful). https://codereview.chromium.org/230443004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Add support for --raw and --omit to js2c. (issue 225723002)

2014-04-10 Thread vogelheim
Committed patchset #3 manually as r20657 (presubmit successful). https://codereview.chromium.org/225723002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Support external startup data in V8. (retry) (issue 334913004 by vogelh...@chromium.org)

2014-06-13 Thread vogelheim
Reviewers: jochen traveling until Jun 23, Description: Support external startup data in V8. [Re-retry of r21696 and r21739] If the embedder chooses, the 'natives' (library sources) and the precompiled startup blob can be written to files during the build process and handed over to V8 at

[v8-dev] Re: Support external startup data in V8. (retry) (issue 334913004 by vogelh...@chromium.org)

2014-06-23 Thread vogelheim
Committed patchset #3 manually as r21941 (presubmit successful). https://codereview.chromium.org/334913004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Fix #include order in snapshot-external.cc (issue 351643002 by jacob.bram...@arm.com)

2014-06-23 Thread vogelheim
lgtm Thanks! https://codereview.chromium.org/351643002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails

[v8-dev] Build snapshot_blob.bin only for the host when using separate toolsets. (issue 353143002 by vogelh...@chromium.org)

2014-06-27 Thread vogelheim
Reviewers: jochen, Description: Build snapshot_blob.bin only for the host when using separate toolsets. BUG=389310 Please review this at https://codereview.chromium.org/353143002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+5, -2 lines): M

[v8-dev] Re: Build snapshot_blob.bin only for the host when using separate toolsets. (issue 353143002 by vogelh...@chromium.org)

2014-06-27 Thread vogelheim
Committed patchset #1 manually as r22068 (presubmit successful). https://codereview.chromium.org/353143002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Introduce code serializer/deserializer. (issue 373713006 by yang...@chromium.org)

2014-07-07 Thread vogelheim
lgtm A few nitpicks, and honestly I'm not sure if all of them make any sense. Feel free to proceed as you see fit... :) https://codereview.chromium.org/373713006/diff/1/src/compiler.cc File src/compiler.cc (right): https://codereview.chromium.org/373713006/diff/1/src/compiler.cc#newcode954

[v8-dev] Re: Fix issues with code serializer. (issue 379563002 by yang...@chromium.org)

2014-07-08 Thread vogelheim
lgtm https://codereview.chromium.org/379563002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Refactor ScriptData class for cached compile data. (issue 376223002 by yang...@chromium.org)

2014-07-09 Thread vogelheim
lgtm Mostly nitpicks... I do think the crash-on-CacheData-from-the-wrong-version thing needs to be documented. https://codereview.chromium.org/376223002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: Refactor ScriptData class for cached compile data. (issue 376223002 by yang...@chromium.org)

2014-07-09 Thread vogelheim
lgtm Mostly nitpicks... I do think the crash-on-CacheData-from-the-wrong-version thing needs to be documented. https://codereview.chromium.org/376223002/diff/1/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/376223002/diff/1/src/parser.cc#newcode267

[v8-dev] Re: Refactor ScriptData class for cached compile data. (issue 376223002 by yang...@chromium.org)

2014-07-10 Thread vogelheim
lgtm https://codereview.chromium.org/376223002/diff/60001/src/preparse-data.h File src/preparse-data.h (right): https://codereview.chromium.org/376223002/diff/60001/src/preparse-data.h#newcode122 src/preparse-data.h:122: class ScriptData; stlye nitpick: Forward declarations near always

[v8-dev] Change ScriptCompiler::CompileOptions and add d8 --cache. (issue 389573006 by vogelh...@chromium.org)

2014-07-11 Thread vogelheim
Reviewers: Yang, marja, Message: The full change, as previously discussed. Does this make sense, overall? Description: Change ScriptCompiler::CompileOptions to allow for two 'cache' modes (parser or code) and to be explicit about cache consumption or production (rather than making presence of

[v8-dev] Re: Change ScriptCompiler::CompileOptions and add d8 --cache. (issue 389573006 by vogelh...@chromium.org)

2014-07-11 Thread vogelheim
+ulan, because API change. https://codereview.chromium.org/389573006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop

[v8-dev] Re: Change ScriptCompiler::CompileOptions and add d8 --cache. (issue 389573006 by vogelh...@chromium.org)

2014-07-14 Thread vogelheim
https://codereview.chromium.org/389573006/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/389573006/diff/1/src/api.cc#newcode1755 src/api.cc:1755: script_data != NULL) { On 2014/07/14 07:45:46, marja wrote: I don't think it's possible that (options ==

[v8-dev] Re: Change ScriptCompiler::CompileOptions and add d8 --cache. (issue 389573006 by vogelh...@chromium.org)

2014-07-15 Thread vogelheim
Minor fixes to tests. Added emulation of the old enum values so that embedders have a chance to adapt. I'm currently wrestling with the trybots; will commit soon-ish unless someone screams. https://codereview.chromium.org/389573006/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Verify that source string matches serialized code. (issue 394793002 by yang...@chromium.org)

2014-07-15 Thread vogelheim
lgtm https://codereview.chromium.org/394793002/diff/1/src/objects.h File src/objects.h (right): https://codereview.chromium.org/394793002/diff/1/src/objects.h#newcode8867 src/objects.h:8867: }; design nitpick: Not sure if exposing this class is the best choice. Given that - because of the

[v8-dev] Re: Verify that source string matches serialized code. (issue 394793002 by yang...@chromium.org)

2014-07-15 Thread vogelheim
lgtm Neat! That's nicer than the old implementation. :) https://codereview.chromium.org/394793002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Reduce amount of boilerplate in assert-scope.h (issue 396923002 by yang...@chromium.org)

2014-07-16 Thread vogelheim
lgtm This does reduce boilerplate, but honestly I'm not sure whether it's really an improvement. This is way deep in 'bikeshed' territory, so please decide as you see fit. My concern is this: When you recently used one of these asserts, I spent quite a bit of time reading through the

[v8-dev] Re: Change ScriptCompiler::CompileOptions and add d8 --cache. (issue 389573006 by vogelh...@chromium.org)

2014-07-16 Thread vogelheim
Committed patchset #5 manually as r22431 (presubmit successful). https://codereview.chromium.org/389573006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Implement --omit, --raw_[context_]file=... for mksnapshot tool. (issue 249283002)

2014-04-23 Thread vogelheim
Reviewers: jochen, Sven Panne, Message: Next steps would be: 1, add a variable to gyp files to generate raw files. 2, add logic in Chromium to (statically) use these. 3, put the runtime part of mksnapshot behind an API (essentially, rip out SnapshotWriter), so this can (optionally) be

[v8-dev] Re: Implement --omit, --raw_[context_]file=... for mksnapshot tool. (issue 249283002)

2014-04-24 Thread vogelheim
PTAL. Not sure what to do abount the flags thing, though. https://codereview.chromium.org/249283002/diff/20001/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/249283002/diff/20001/src/flag-definitions.h#newcode660 src/flag-definitions.h:660:

[v8-dev] Re: Implement --omit, --raw_[context_]file=... for mksnapshot tool. (issue 249283002)

2014-04-24 Thread vogelheim
: DEFINE_bool(omit, false, Omit raw snapshot bytes in generated code.) On 2014/04/24 11:07:59, vogelheim wrote: [...] Honestly, I'm not super fond of this design, but I can't see any fix other than refactoring the whole flags thing. I suspected that... :-/ OK, then just add (mksnapshot only

[v8-dev] Re: Implement --omit, --raw_[context_]file=... for mksnapshot tool. (issue 249283002)

2014-04-24 Thread vogelheim
Committed patchset #3 manually as r20941 (presubmit successful). https://codereview.chromium.org/249283002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Add defensive assert for having a weak, empty presistent in a (issue 265823006)

2014-05-06 Thread vogelheim
Reviewers: jochen, Message: PTAL. That first version wasn't meant for review, as both you and the compiler noticed.. :-P This triggers a minor bug in the webkit tests, so I'll have to wait for that to be fixed before submitting. Description: Add defensive assert for having a weak, empty

[v8-dev] Prevent calls to ReturnValue::Set with pointer-valued types. (issue 240013004)

2014-05-09 Thread vogelheim
Reviewers: dcarney, Message: Please have a look. (Following crrev.com/267393002, this now compiles just fine in Chromium + blink.) Description: Prevent calls to ReturnValue::Set with pointer-valued types. Currently, this code will compile: SomePointer* p = ...; ReturnValue r = ...;

[v8-dev] Re: Prevent calls to ReturnValue::Set with pointer-valued types. (issue 240013004)

2014-05-09 Thread vogelheim
Committed patchset #1 manually as r21217 (presubmit successful). https://codereview.chromium.org/240013004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Prevent calls to ReturnValue::Set with pointer-valued types. (issue 240013004)

2014-05-09 Thread vogelheim
A revert of this CL has been created in https://codereview.chromium.org/271113002/ by vogelh...@chromium.org. The reason for reverting is: Looks like this broke the V8 Linux64 ASAN build.. https://codereview.chromium.org/240013004/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Revert of Prevent calls to ReturnValue::Set with pointer-valued types. (issue 271113002)

2014-05-09 Thread vogelheim
Reviewers: dcarney, Message: Created Revert of Prevent calls to ReturnValue::Set with pointer-valued types. Description: Revert of Prevent calls to ReturnValue::Set with pointer-valued types. (https://codereview.chromium.org/240013004/) Reason for revert: Looks like this broke the V8

[v8-dev] Re: Revert of Prevent calls to ReturnValue::Set with pointer-valued types. (issue 271113002)

2014-05-09 Thread vogelheim
Committed patchset #1 manually as r21219 (tree was closed). https://codereview.chromium.org/271113002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Prevent calls to ReturnValue::Set with pointer-valued types. (issue 279883002)

2014-05-09 Thread vogelheim
Reviewers: dcarney, Description: Prevent calls to ReturnValue::Set with pointer-valued types. [2nd try, after the previous version broke the build] Currently, this code will compile: SomePointer* p = ...; ReturnValue r = ...; r.Set(p); What happens is that ReturnValue::Set has no pointer-ish

[v8-dev] Re: Prevent calls to ReturnValue::Set with pointer-valued types. (issue 279883002)

2014-05-09 Thread vogelheim
Committed patchset #1 manually as r21228 (presubmit successful). https://codereview.chromium.org/279883002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Support external startup data in V8. (issue 293993021)

2014-05-26 Thread vogelheim
Reviewers: jochen, https://codereview.chromium.org/293993021/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/293993021/diff/1/include/v8.h#newcode4668 include/v8.h:4668: #ifdef V8_USE_EXTERNAL_STARTUP_DATA On 2014/05/23 11:44:44, jochen wrote: embedders won't

[v8-dev] Fix the PersistentValueMap memory leak reported here: (issue 297193004)

2014-05-26 Thread vogelheim
Reviewers: dcarney, Description: Fix the PersistentValueMap memory leak reported here: http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20ASAN The bug: The code assumed that a weak Persistent whose weak callback is being called would still be weak. That isn't true since the

[v8-dev] Re: Fix the PersistentValueMap memory leak reported here: (issue 297193004)

2014-05-27 Thread vogelheim
Committed patchset #1 manually as r21514 (presubmit successful). https://codereview.chromium.org/297193004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Add defensive assert for having a weak, empty presistent in a (issue 265823006)

2014-05-27 Thread vogelheim
Committed patchset #2 manually as r21521 (presubmit successful). https://codereview.chromium.org/265823006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Support external startup data in V8. (issue 293993021)

2014-05-27 Thread vogelheim
PTAL. (Had to rebase, so unfortunately the diffs between versions are hard to read. Is there any way around that?) https://codereview.chromium.org/293993021/diff/40001/src/natives-external.cc File src/natives-external.cc (right):

[v8-dev] Re: Support external startup data in V8. (issue 293993021)

2014-06-03 Thread vogelheim
Committed patchset #6 manually as r21646 (presubmit successful). https://codereview.chromium.org/293993021/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: Support external startup data in V8. (issue 315033002)

2014-06-05 Thread vogelheim
Reviewers: jochen, Message: Committed patchset #2 manually as r21696 (presubmit successful). Description: Support external startup data in V8. [Retry of crrev.com/293993021, which caused problems with 'ninja all' in Chromium. First patch set if a clean apply of crrev.com/293993021. Subsequent

[v8-dev] Re: Fix presubmit warning. (issue 319563003)

2014-06-05 Thread vogelheim
Reviewers: mvstanton, Message: Committed patchset #1 manually as r21699 (presubmit successful). Description: Fix presubmit warning. (http://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/8147/steps/Presubmit/logs/stdio) R=mvstan...@chromium.org Committed:

[v8-dev] Port 'external startup data' flag from gyp to gn. (issue 316363004)

2014-06-06 Thread vogelheim
Reviewers: jochen traveling until Jun 23, Description: Port 'external startup data' flag from gyp to gn. R=joc...@chromium.org BUG= Please review this at https://codereview.chromium.org/316363004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+92, -12 lines):

[v8-dev] Re: Port 'external startup data' flag from gyp to gn. (issue 316363004)

2014-06-06 Thread vogelheim
Committed patchset #2 manually as r21721 (presubmit successful). https://codereview.chromium.org/316363004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To

[v8-dev] Re: d8: create compile cache in a separate isolate. (issue 670433003 by yang...@chromium.org)

2014-10-20 Thread vogelheim
lgtm Thanks! Looks nicer this way... :) https://codereview.chromium.org/670433003/diff/1/src/d8.cc File src/d8.cc (right): https://codereview.chromium.org/670433003/diff/1/src/d8.cc#newcode180 src/d8.cc:180: ScriptCompiler::CachedData* Shell::CompileCachedData( nitpick: I suspect this

[v8-dev] Re: De-virtualize snapshot sink. (issue 669133003 by yang...@chromium.org)

2014-10-22 Thread vogelheim
lgtm https://codereview.chromium.org/669133003/diff/1/src/snapshot-source-sink.h File src/snapshot-source-sink.h (right): https://codereview.chromium.org/669133003/diff/1/src/snapshot-source-sink.h#newcode74 src/snapshot-source-sink.h:74: explicit SnapshotByteSink(int initial_size) :

[v8-dev] Re: Make block writes in the serializer more efficient. (issue 671633004 by yang...@chromium.org)

2014-10-23 Thread vogelheim
lgtm https://codereview.chromium.org/671633004/diff/1/src/serialize.cc File src/serialize.cc (right): https://codereview.chromium.org/671633004/diff/1/src/serialize.cc#newcode1635 src/serialize.cc:1635: for (int i = 0; i padding_size; i++) sink_-PutSection(0, StringPadding); If I get this

[v8-dev] Re: Make block writes in the serializer more efficient. (issue 671633004 by yang...@chromium.org)

2014-10-23 Thread vogelheim
On 2014/10/23 15:51:40, Yang wrote: note to self. use Lis::Resize and memcpy instead of List::AddAll Maybe do that inside List::AddAll? If this is a benefit for our case, it's likely a benefit for all callers. https://codereview.chromium.org/671633004/ -- -- v8-dev mailing list

[v8-dev] Re: Enable --serialize-toplevel by default. (issue 661343003 by yang...@chromium.org)

2014-10-24 Thread vogelheim
lgtm https://codereview.chromium.org/661343003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)

2014-11-12 Thread vogelheim
Reviewers: marja, dcarney, Message: Does this version of the API make any sense? Also, any ideas for unit tests? I'll add one that the tag changes when the flag list changes; but I don't see how I can easily unit test any of the other issues. Description: Add a version tag for cached data.

[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)

2014-11-12 Thread vogelheim
On 2014/11/12 14:49:27, dcarney wrote: lgtm, a simple unit test is really all you can do here Yeah. And I'm having difficulty with changing the command line flags in cctest. If I can't resolve this tomorrow, I'll commit without unit test. https://codereview.chromium.org/718043002/ -- --

[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)

2014-11-12 Thread vogelheim
https://codereview.chromium.org/718043002/diff/20001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/718043002/diff/20001/src/api.cc#newcode1851 src/api.cc:1851: internal::CpuFeatures::SupportedFeatures(); On 2014/11/12 15:55:31, Sven Panne wrote: This is a very bad hash,

[v8-dev] Re: Soft fail for invalid cache data. (issue 724023002 by yang...@chromium.org)

2014-11-13 Thread vogelheim
https://codereview.chromium.org/724023002/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/724023002/diff/1/src/api.cc#newcode1759 src/api.cc:1759: source-cached_data-rejected = (script_data == NULL); I find this a bit weird... - I expected ..-rejected to be set at

[v8-dev] Re: Soft fail for invalid cache data. (issue 724023002 by yang...@chromium.org)

2014-11-13 Thread vogelheim
https://codereview.chromium.org/724023002/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/724023002/diff/1/src/api.cc#newcode1759 src/api.cc:1759: source-cached_data-rejected = (script_data == NULL); If ScriptData is the internal structure (vs CachedData as an API

  1   2   3   4   >