[v8-dev] Re: Version 4.3.61.12 (cherry-pick) (issue 1055603004 by hpa...@chromium.org)

2015-04-23 Thread rmcilroy
On 2015/04/23 12:50:58, Hannes Payer wrote: merge conflict resolved Lgtm https://codereview.chromium.org/1055603004/ -- -- 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

[v8-dev] Re: Version 4.2.77.18 (cherry-pick) (issue 1103683002 by hpa...@chromium.org)

2015-04-23 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1103683002/diff/20001/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1103683002/diff/20001/src/heap/heap.h#newcode1616 src/heap/heap.h:1616: // The allocation limit when there is 16.66ms idle time in the idle time

[v8-dev] Re: Replace OVERRIDE-override and FINAL-final since we now require C++11. (issue 1088993003 by rmcil...@chromium.org)

2015-04-20 Thread rmcilroy
Committed patchset #1 (id:1) manually as 063fc25122d2e569785587ff1bd13fa5c3e185ba (presubmit successful). https://codereview.chromium.org/1088993003/ -- -- 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] Replace OVERRIDE-override and FINAL-final since we now require C++11. (issue 1088993003 by rmcil...@chromium.org)

2015-04-20 Thread rmcilroy
Reviewers: jochen, Message: Jochen: could you please take a look, thanks. I would appreciate a quick review please, to avoid too much rebase pain :). Main changes are in v8config.h and compiler_support.h - the rest should just be a mechanical change renaming override and final in

[v8-dev] Re: Avoid executing constant pool related code paths in MarkCompact when constant pool support is disab… (issue 1096053002 by ish...@chromium.org)

2015-04-20 Thread rmcilroy
On 2015/04/20 17:19:42, Jakob wrote: I'm deferring to Ross here, who judging by https://codereview.chromium.org/179813005 is more familiar with this code than I am :-) Now that I think about it, let's just delete both constant pool branches (i.e., L4611-4623) and always add typed slots

[v8-dev] Turn off SupportsFlexibleFloorAndRound for Arm64 due to a bug. (issue 1093863002 by rmcil...@chromium.org)

2015-04-17 Thread rmcilroy
{ // Math.round. bool SupportsFlexibleFloorAndRound() const { #ifdef V8_TARGET_ARCH_ARM64 -return true; +// TODO(rmcilroy): Re-enable this for Arm64 once http://crbug.com/476477 is +// fixed. +return false; #else return false; #endif -- -- v8-dev mailing list v8-dev

[v8-dev] Re: Use smaller heap growing factor in idle notification to start incremental marking when there is idl… (issue 1090963002 by hpa...@chromium.org)

2015-04-16 Thread rmcilroy
On 2015/04/16 15:00:03, ulan wrote: lgtm lgtm, thanks. https://codereview.chromium.org/1090963002/ -- -- 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: Enable constant pool support. (issue 1030353003 by michael_daw...@ca.ibm.com)

2015-04-16 Thread rmcilroy
On 2015/04/16 15:56:39, mtbrandyberry wrote: On 2015/04/08 12:38:55, rmcilroy wrote: Apologies for the delay - we were working out what we want to do with the current OOL constant pool implementation. The high level feedback is that we don't want to have yet another separate

[v8-dev] Re: Version 4.3.61.2 (cherry-pick) (issue 1071503002 by rmcil...@chromium.org)

2015-04-08 Thread rmcilroy
Committed patchset #1 (id:1) manually as 79e7295a8c0959caa48d6f98824842fa62d43b5e. https://codereview.chromium.org/1071503002/ -- -- 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] Re: When sweeping is in progress, either finalize or do nothing (issue 1070653003 by joc...@chromium.org)

2015-04-08 Thread rmcilroy
https://codereview.chromium.org/1070653003/diff/1/src/heap/gc-idle-time-handler.cc File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1070653003/diff/1/src/heap/gc-idle-time-handler.cc#newcode260 src/heap/gc-idle-time-handler.cc:260: return

[v8-dev] Version 4.3.61.2 (cherry-pick) (issue 1071503002 by rmcil...@chromium.org)

2015-04-08 Thread rmcilroy
Reviewers: Hannes Payer, Description: Version 4.3.61.2 (cherry-pick) Merged 00477a5d72e1246d9611966a0b4c708f90a2228e Ensure that GC idle notifications either make progress or stop requesting more GCs. BUG=chromium:470615 LOG=N R=hpa...@chromium.org Please review this at

[v8-dev] Re: When sweeping is in progress, either finalize or do nothing (issue 1070653003 by joc...@chromium.org)

2015-04-08 Thread rmcilroy
lgtm, thanks! https://codereview.chromium.org/1070653003/ -- -- 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

[v8-dev] Re: Enable constant pool support. (issue 1030353003 by michael_daw...@ca.ibm.com)

2015-04-08 Thread rmcilroy
Apologies for the delay - we were working out what we want to do with the current OOL constant pool implementation. The high level feedback is that we don't want to have yet another separate configuration for constant pools, however we would be OK with supporting an embedded constant pool and

[v8-dev] Fix libdl dependency on Android and remove librt hack. (issue 1036133005 by rmcil...@chromium.org)

2015-03-31 Thread rmcilroy
Reviewers: jochen, Benedikt Meurer (OOO), jarin, Message: jochen for libdl fix bmeurer for librt hack removal jarin as FYI for perf-jit comment removal. PTAL, thanks! Description: Fix libdl dependency on Android and remove librt hack. The libdl library is already included on target builds of

[v8-dev] Re: Finalize sweeping in idle notification when all pages are swept. (issue 1038283003 by hpa...@chromium.org)

2015-03-30 Thread rmcilroy
On 2015/03/30 10:05:44, I haz the power (commit-bot) wrote: Patchset 4 (id:??) landed as https://crrev.com/eda9a88f2f941d4ef84f2e48e5309b5ab156d7af Cr-Commit-Position: refs/heads/master@{#27513} nice, lgtm, thanks! https://codereview.chromium.org/1038283003/ -- -- v8-dev mailing list

[v8-dev] Re: Ensure that GC idle notifications either make progress or stop requesting more GCs. (issue 1042483002 by rmcil...@chromium.org)

2015-03-30 Thread rmcilroy
On 2015/03/30 08:31:17, Hannes Payer wrote: OK, sounds good. Please add a unit test. Done https://codereview.chromium.org/1042483002/ -- -- 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

[v8-dev] Re: Allow more scavenges in idle notification by increasing the new space limit distance. (issue 1034403002 by hpa...@chromium.org)

2015-03-30 Thread rmcilroy
-handler.cc:136: kMaxScheduledIdleTime * 2; On 2015/03/27 15:07:50, rmcilroy wrote: Pull out 'kMaxScheduledIdleTime * 2' as a variable named something like estimated_time_till_next_idle_notification or similar and add a comment you are estimating that it will be 2 * frame time since we might

[v8-dev] Re: Allow more scavenges in idle notification by increasing the new space limit distance. (issue 1034403002 by hpa...@chromium.org)

2015-03-27 Thread rmcilroy
https://codereview.chromium.org/1034403002/diff/1/src/heap/gc-idle-time-handler.cc File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1034403002/diff/1/src/heap/gc-idle-time-handler.cc#newcode136 src/heap/gc-idle-time-handler.cc:136: kMaxScheduledIdleTime * 2; Pull

[v8-dev] Ensure that GC idle notifications either make progress or stop requesting more GCs. (issue 1042483002 by rmcil...@chromium.org)

2015-03-27 Thread rmcilroy
Reviewers: Hannes Payer, Message: PTAL, thanks. Description: Ensure that GC idle notifications either make progress or stop requesting more GCs. The V8::IdleNotification will only return 'True' when the gc idle time handler thinks there is no more GC which can be done. However, the gc

[v8-dev] Re: Re-work the 'external snapshot' related build rules. (issue 1016603004 by vogelh...@chromium.org)

2015-03-27 Thread rmcilroy
lgtm. https://codereview.chromium.org/1016603004/ -- -- 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: Ensure that GC idle notifications either make progress or stop requesting more GCs. (issue 1042483002 by rmcil...@chromium.org)

2015-03-27 Thread rmcilroy
On 2015/03/27 19:04:19, Hannes Payer wrote: What about the following scenario: We just finished a full gc and it while take a while to start incremental marking. We continuously approach the incremental marking start state but are slower then 10 received idle notifications. In that case we

[v8-dev] Re: Simplified garbage collection idle handler. (issue 1024043003 by hpa...@chromium.org)

2015-03-24 Thread rmcilroy
On 2015/03/23 14:38:39, Erik Corry wrote: It's a good simplification. We have 3 triggers for a GC when this lands: IncrementalMarking: kActivationThreshold: 8Mbytes promoted GCIdleTimeHandler: KIdleScavengeThreshold: 5 new-space GCs Heap::OldGenerationAllocationLimit: Growth of 1.1x to 4x

[v8-dev] Re: Limit rate of full garbage collections triggered by idle notification. (issue 1023153002 by hpa...@chromium.org)

2015-03-20 Thread rmcilroy
looks good overall - couple of questions though. https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.cc File src/heap/gc-idle-time-handler.cc (right): https://codereview.chromium.org/1023153002/diff/80001/src/heap/gc-idle-time-handler.cc#newcode162

[v8-dev] Re: Revert of Revert of Turn on job based recompilation (issue 955723002 by joc...@chromium.org)

2015-03-06 Thread rmcilroy
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/984463003/ by rmcil...@chromium.org. The reason for reverting is: Breaks Octane on Arm64 running on Chrome. BUG=464538. https://codereview.chromium.org/955723002/ -- -- v8-dev mailing list

[v8-dev] Revert of Revert of Revert of Turn on job based recompilation (issue 984463003 by rmcil...@chromium.org)

2015-03-06 Thread rmcilroy
Reviewers: Yang, jochen (traveling), Message: Created Revert of Revert of Revert of Turn on job based recompilation Description: Revert of Revert of Revert of Turn on job based recompilation (patchset #1 id:1 of https://codereview.chromium.org/955723002/) Reason for revert: Breaks Octane on

[v8-dev] Re: Don't apply pointer multipler to heap sizes on Android. (issue 960213007 by rmcil...@chromium.org)

2015-03-03 Thread rmcilroy
On 2015/03/02 22:04:27, Hannes Payer wrote: LGTM, this change will tank 64-bit android devices. Thanks. One thing we could do if this does tank 64-bit android too much is use the pointer multiplier to increase the semi-space size, but not the old-space. Would this help do you think?

[v8-dev] Re: Move stack unwinding logic into the runtime. (issue 960273002 by mstarzin...@chromium.org)

2015-03-03 Thread rmcilroy
On 2015/03/02 15:08:49, Michael Starzinger wrote: Thanks for the feedback. Now all I have to fix is Win64. :( https://codereview.chromium.org/960273002/diff/110020/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right):

[v8-dev] Don't apply pointer multipler to heap sizes on Android. (issue 960213007 by rmcil...@chromium.org)

2015-03-02 Thread rmcilroy
Reviewers: Hannes Payer, Message: Hannes: PTAL, thanks. This is to fix crbug.com/432909 Description: Don't apply pointer multipler to heap sizes on Android. Android doesn't have swap space so if the heap goes over the physical memory size the system will just kill us. Applying the

[v8-dev] Re: Move stack unwinding logic into the runtime. (issue 960273002 by mstarzin...@chromium.org)

2015-03-02 Thread rmcilroy
Arm code looks reasonable other than use of 'ip' register. https://codereview.chromium.org/960273002/diff/80001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): https://codereview.chromium.org/960273002/diff/80001/src/arm/code-stubs-arm.cc#newcode964

[v8-dev] Re: Protect access to the external_snapshot_blob global with a lock. (issue 918213002 by rmcil...@chromium.org)

2015-02-13 Thread rmcilroy
Committed patchset #1 (id:1) manually as 1da1e34788eca083eec26492e16da76f8520eee4 (presubmit successful). https://codereview.chromium.org/918213002/ -- -- 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] Protect access to the external_snapshot_blob global with a lock. (issue 918213002 by rmcil...@chromium.org)

2015-02-12 Thread rmcilroy
Reviewers: vogelheim, Message: PTAL, thanks. Description: Protect access to the external_snapshot_blob global with a lock. The external_snapshot_blob is a global and might be accessed from multiple threads. Protect it with a lock. BUG=457656 LOG=N Please review this at

[v8-dev] Re: Contribution of PowerPC port (continuation of 422063005) - PPC opt 1 (issue 882263003 by michael_daw...@ca.ibm.com)

2015-02-10 Thread rmcilroy
it's currently disabled because it (a) tanks performance This is not quite true - it regressed Splay.Latency (~20%), but was performance neutral on the other benchmarks on Arm and the overall regression on Octane was pretty small (~1-2%). It was enabled for quite some time on Arm without

[v8-dev] Re: Version 4.1.0.5 (cherry-pick) (issue 838383007 by joc...@chromium.org)

2015-01-13 Thread rmcilroy
On 2015/01/13 10:06:57, jochen (slow) wrote: Committed patchset #1 (id:1) manually as 9fcb3ee133376fd601d13aa091de3bf8d4eb20f3. lgtm, thanks. https://codereview.chromium.org/848443002/ (pdfium) will also need to be cherry-picked once it lands. https://codereview.chromium.org/838383007/ --

[v8-dev] Re: [d8] Fix V8 external snapshot for Windows. (issue 842573004 by rmcil...@chromium.org)

2015-01-12 Thread rmcilroy
On 2015/01/12 12:16:29, jochen (slow) wrote: lgtm Thanks! https://codereview.chromium.org/842573004/ -- -- 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] [d8] Fix V8 external snapshot for Windows. (issue 842573004 by rmcil...@chromium.org)

2015-01-09 Thread rmcilroy
Reviewers: jochen (slow), Message: This is required to land https://codereview.chromium.org/826083004/. PTAL, thanks. Description: [d8] Fix V8 external snapshot for Windows. Adds a cast to V8 external snapshot code for Windows. BUG=421063, 439661 LOG=N Please review this at

[v8-dev] Re: Make FlushICache NOP for Nvidia Denver CPU's. (issue 797233002 by ar...@nvidia.com)

2014-12-18 Thread rmcilroy
https://codereview.chromium.org/797233002/diff/20001/src/arm64/assembler-arm64.cc File src/arm64/assembler-arm64.cc (right): https://codereview.chromium.org/797233002/diff/20001/src/arm64/assembler-arm64.cc#newcode63 src/arm64/assembler-arm64.cc:63: void CpuFeatures::PrintFeatures() { } Please

[v8-dev] Re: Make FlushICache NOP for Nvidia Denver CPU's. (issue 797233002 by ar...@nvidia.com)

2014-12-18 Thread rmcilroy
On 2014/12/18 14:05:07, rmcilroy wrote: https://codereview.chromium.org/797233002/diff/20001/src/arm64/assembler-arm64.cc File src/arm64/assembler-arm64.cc (right): https://codereview.chromium.org/797233002/diff/20001/src/arm64/assembler-arm64.cc#newcode63 src/arm64/assembler-arm64.cc:63

[v8-dev] Re: Phantom references support internal fields (issue 753553002 by erikco...@chromium.org)

2014-12-16 Thread rmcilroy
On 2014/12/15 15:39:40, jochen (slow) wrote: lgtm assuming you can make the compilers happy :) lgtm, thanks. https://codereview.chromium.org/753553002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] [GN] Output external snapshot blobs in out directory. (issue 805813004 by rmcil...@chromium.org)

2014-12-15 Thread rmcilroy
Reviewers: jochen (slow), Message: Jochen, PTAL, thanks. Description: [GN] Output external snapshot blobs in out directory. The snapshot and natives blob files should be output in the out directory instead of the gen directory so that they can be picked up by the executable. BUG=421063 LOG=N

[v8-dev] Re: Make FlushICache NOP for Nvidia Denver CPU's. (issue 797233002 by ar...@nvidia.com)

2014-12-15 Thread rmcilroy
https://codereview.chromium.org/797233002/diff/1/src/arm64/assembler-arm64.cc File src/arm64/assembler-arm64.cc (right): https://codereview.chromium.org/797233002/diff/1/src/arm64/assembler-arm64.cc#newcode48 src/arm64/assembler-arm64.cc:48: supported_ = 0; On 2014/12/15 16:23:23, JF wrote:

[v8-dev] Fix OS::GetCurrentThreadId to work when building Android on Mac. (issue 799943003 by rmcil...@chromium.org)

2014-12-12 Thread rmcilroy
Reviewers: jochen (slow), Message: Jochen: could you please take a look at this CL. Description: Fix OS::GetCurrentThreadId to work when building Android on Mac. The Mac version of GetCurrentThreadId should be used when building the host build of V8 on Android for Mac. Please review this at

[v8-dev] Re: move v8_use_external_startup_data to standalone.gypi (issue 794583002 by most...@opera.com)

2014-12-11 Thread rmcilroy
https://codereview.chromium.org/794583002/diff/1/build/features.gypi File build/features.gypi (left): https://codereview.chromium.org/794583002/diff/1/build/features.gypi#oldcode100 build/features.gypi:100: }], This needs to still be here since we want to define V8_USE_EXTERNAL_STARTUP_DATA

[v8-dev] Re: Phantom references support internal fields (issue 753553002 by erikco...@chromium.org)

2014-12-03 Thread rmcilroy
https://codereview.chromium.org/753553002/diff/80001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/753553002/diff/80001/include/v8.h#newcode422 include/v8.h:422: class CallbackData { nit - BaseCallbackData or maybe even BaseWeakCallbackData? Would it make sense to put

[v8-dev] Re: Use deadline in IdleNotification. (issue 750813003 by hpa...@chromium.org)

2014-11-26 Thread rmcilroy
Lgtm with a couple of final comments. Also - you could link this with crbug.com/417668, which I've just made you owner of :). https://codereview.chromium.org/750813003/diff/310001/src/heap/gc-idle-time-handler.cc File src/heap/gc-idle-time-handler.cc (right):

[v8-dev] Re: Ensure double alignment when deserializing. (issue 759823006 by yang...@chromium.org)

2014-11-26 Thread rmcilroy
lgtm, with some comments. Thanks for doing this Yang! https://codereview.chromium.org/759823006/diff/40001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/759823006/diff/40001/src/heap/heap.cc#newcode1833 src/heap/heap.cc:1833: return EnsureDoubleAligned(this,

[v8-dev] Re: Ensure double alignment when deserializing. (issue 759823006 by yang...@chromium.org)

2014-11-26 Thread rmcilroy
: On 2014/11/26 17:44:40, rmcilroy wrote: Could you add an ASSERT(object-NeedsToEnsureDoubleAlignment()) in EnsureDoubleAligned to ensure that the list in NeedsToEnsureDoubleAlignment keeps in sync with objects which call EnsureDoubleAligned during allocation. Unfortunately, at this point

[v8-dev] Re: Ensure double alignment when deserializing. (issue 759823006 by yang...@chromium.org)

2014-11-26 Thread rmcilroy
https://codereview.chromium.org/759823006/ -- -- 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 it, send

[v8-dev] Re: Ensure double alignment when deserializing. (issue 759823006 by yang...@chromium.org)

2014-11-26 Thread rmcilroy
https://codereview.chromium.org/759823006/ -- -- 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 it, send

[v8-dev] Re: Ensure double alignment when deserializing. (issue 759823006 by yang...@chromium.org)

2014-11-26 Thread rmcilroy
https://codereview.chromium.org/759823006/ -- -- 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 it, send

[v8-dev] Re: Use deadline in IdleNotification. (issue 750813003 by hpa...@chromium.org)

2014-11-25 Thread rmcilroy
https://codereview.chromium.org/750813003/diff/120001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/750813003/diff/120001/src/heap/heap.cc#newcode4429 src/heap/heap.cc:4429: deadline - V8::GetCurrentPlatform()-MonotonicallyIncreasingTime(); Both deadline and

[v8-dev] Re: Let Chromium decide whether external V8 snapshot will be used or not. (issue 749213004 by ba...@chromium.org)

2014-11-25 Thread rmcilroy
On 2014/11/24 21:35:13, baixo1 wrote: Addressed Ross' comments. https://codereview.chromium.org/749213004/diff/1/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/749213004/diff/1/BUILD.gn#newcode1418 BUILD.gn:1418: deps = [] On 2014/11/24 18:01:34, rmcilroy wrote: On 2014

[v8-dev] Re: Use deadline in IdleNotification. (issue 750813003 by hpa...@chromium.org)

2014-11-25 Thread rmcilroy
https://codereview.chromium.org/750813003/diff/130001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/750813003/diff/130001/include/v8.h#newcode5001 include/v8.h:5001: * done within the time limit. nit - mention that deadline_seconds is compared with

[v8-dev] Re: Fix alignment of Code::kConstantPoolOffset. (issue 740443002 by andrew_...@ca.ibm.com)

2014-11-24 Thread rmcilroy
On 2014/11/24 07:11:33, Sven Panne wrote: Delegating to our constant pool black belts... ;-) lgtm, thanks. https://codereview.chromium.org/740443002/ -- -- 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: Generate snapshot_blob_host.bin for the host toolset. (issue 741223002 by ba...@chromium.org)

2014-11-24 Thread rmcilroy
On 2014/11/21 20:36:08, baixo1 wrote: On 2014/11/21 13:40:51, rmcilroy wrote: On 2014/11/21 12:34:02, Andrew Hayden wrote: I'm not too worried about the approach; it's not pretty, but GYP is obtuse with things like this. More reasons to get the GN migration done, IMO

[v8-dev] Re: Let Chromium decide whether external V8 snapshot will be used or not. (issue 749213004 by ba...@chromium.org)

2014-11-24 Thread rmcilroy
https://codereview.chromium.org/749213004/diff/1/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/749213004/diff/1/BUILD.gn#newcode1418 BUILD.gn:1418: deps = [] Is there any reason you need to make these changes? I would have thought the if/else if/else structure before would

[v8-dev] Re: Let Chromium decide whether external V8 snapshot will be used or not. (issue 749213004 by ba...@chromium.org)

2014-11-24 Thread rmcilroy
https://codereview.chromium.org/749213004/diff/1/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/749213004/diff/1/BUILD.gn#newcode1418 BUILD.gn:1418: deps = [] On 2014/11/24 17:28:11, baixo1 wrote: On 2014/11/24 17:21:06, rmcilroy wrote: Is there any reason you need to make

[v8-dev] Re: Generate snapshot_blob_host.bin for the host toolset. (issue 741223002 by ba...@chromium.org)

2014-11-21 Thread rmcilroy
On 2014/11/21 12:34:02, Andrew Hayden wrote: I'm not too worried about the approach; it's not pretty, but GYP is obtuse with things like this. More reasons to get the GN migration done, IMO, not that this should be an excuse for poor code quality. Anyhow: Two thoughts. 1. It would be nice

[v8-dev] Re: Ensure external snapshot is only set once. (issue 746263002 by ba...@chromium.org)

2014-11-21 Thread rmcilroy
On 2014/11/21 18:51:08, baixo1 wrote: Could you please take a look? Thanks! lgtm, with two nits. Thanks. https://codereview.chromium.org/746263002/ -- -- 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: Ensure external snapshot is only set once. (issue 746263002 by ba...@chromium.org)

2014-11-21 Thread rmcilroy
On 2014/11/21 18:51:08, baixo1 wrote: Could you please take a look? Thanks! lgtm, with two nits. Thanks. https://codereview.chromium.org/746263002/ -- -- 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: Ensure external snapshot is only set once. (issue 746263002 by ba...@chromium.org)

2014-11-21 Thread rmcilroy
this time with the comments... https://codereview.chromium.org/746263002/diff/1/src/v8.cc File src/v8.cc (right): https://codereview.chromium.org/746263002/diff/1/src/v8.cc#newcode127 src/v8.cc:127: nit - remove extra newline here

[v8-dev] Re: Fix size_t to int conversion. (issue 727513002 by ba...@chromium.org)

2014-11-13 Thread rmcilroy
Committed patchset #1 (id:1) manually as eeb8782a7f9563000d4f7a5f91dafd41974baf04 (presubmit successful). https://codereview.chromium.org/727513002/ -- -- 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: Perform context disposal garbage collections only if contexts disposals happen at a high rate. (issue 710603003 by hpa...@chromium.org)

2014-11-07 Thread rmcilroy
On 2014/11/07 09:36:56, Hannes Payer wrote: Committed patchset #4 (id:60001) manually as 25208 (presubmit successful). Nice, thanks Hannes! https://codereview.chromium.org/710603003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received

[v8-dev] Re: Arm64: Remove forced csp alignment to 16 byte values for Nvidia chips. (issue 710613002 by rmcil...@chromium.org)

2014-11-07 Thread rmcilroy
On 2014/11/07 08:36:10, Rodolph Perfetta (ARM) wrote: LGTM with one nit, could you leave the SyncSystemStackPointer method even if it is a one liner, it was there before (it is part of the sp handling abstraction and does some check in debug). Done, Thanks for the reviews.

[v8-dev] Re: Arm64: Remove forced csp alignment to 16 byte values for Nvidia chips. (issue 710613002 by rmcil...@chromium.org)

2014-11-07 Thread rmcilroy
Committed patchset #2 (id:40001) manually as 25225 (presubmit successful). https://codereview.chromium.org/710613002/ -- -- 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

[v8-dev] Arm64: Remove forced csp alignment to 16 byte values for Nvidia chips. (issue 710613002 by rmcil...@chromium.org)

2014-11-06 Thread rmcilroy
Reviewers: ulan, Rodolph Perfetta (ARM), Message: PTAL, thanks. Description: Arm64: Remove forced csp alignment to 16 byte values for Nvidia chips. Remove the forced alignment of csp to 16 byte values on Nvidia chips. Benchmarks on current devices show that this is no longer required. Please

[v8-dev] Re: Fix lo_space initialization for external snapshot (issue 684263002 by ba...@chromium.org)

2014-10-29 Thread rmcilroy
lgtm, thanks. +vogelheim / yangguo - could we add some external snapshot tests to ensure that changes to the snapshot code don't break the external snapshot in the future now that we will be using it in Chrome? https://codereview.chromium.org/684263002/ -- -- v8-dev mailing list

[v8-dev] Re: Fix lo_space initialization for external snapshot (issue 684263002 by ba...@chromium.org)

2014-10-29 Thread rmcilroy
Committed patchset #1 (id:1) manually as 24998 (presubmit successful). https://codereview.chromium.org/684263002/ -- -- 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] Re: Disable OOL constant pool for Arm. (issue 663333005 by rmcil...@chromium.org)

2014-10-28 Thread rmcilroy
Committed patchset #1 (id:1) manually as 24953 (presubmit successful). https://codereview.chromium.org/66005/ -- -- 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] Disable OOL constant pool for Arm. (issue 663333005 by rmcil...@chromium.org)

2014-10-27 Thread rmcilroy
Reviewers: ulan, Message: Off again... PTAL, thanks. Description: Disable OOL constant pool for Arm. This caused some performance regressions on Octane. Please review this at https://codereview.chromium.org/66005/ Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge Affected

[v8-dev] Re: Flush code faster when doing emergency GCs. (issue 677043002 by erikco...@chromium.org)

2014-10-24 Thread rmcilroy
On 2014/10/24 11:18:57, ulan wrote: Hi Erik, If I understand correctly, this flushes all flushable code including active code. This could lead to recompile/flush cycles and could actually increase memory usage because of fragmentation in code space. Would this not happen currently for

[v8-dev] Re: Flush code faster when doing emergency GCs. (issue 677043002 by erikco...@chromium.org)

2014-10-24 Thread rmcilroy
Nice, it's always nice when a 2 year old CL comes back :). LGTM, but please wait for Ulan since I don't know the code you are touching in heap-inl.h very well. https://codereview.chromium.org/677043002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right):

[v8-dev] Re: v8_external_snapshot target visible only when using v8_use_external_startup_data (issue 643163002 by ba...@chromium.org)

2014-10-13 Thread rmcilroy
Committed patchset #2 (id:60001) manually as 24550 (presubmit successful). https://codereview.chromium.org/643163002/ -- -- 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

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-10-13 Thread rmcilroy
Committed patchset #5 (id:80001) manually as 24565 (presubmit successful). https://codereview.chromium.org/609843002/ -- -- 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

[v8-dev] Re: v8_external_snapshot target visible only when using v8_use_external_startup_data (issue 643163002 by ba...@chromium.org)

2014-10-11 Thread rmcilroy
LGTM, thanks Andre. https://codereview.chromium.org/643163002/ -- -- 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

[v8-dev] Re: v8_external_snapshot target visible only when using v8_use_external_startup_data (issue 643163002 by ba...@chromium.org)

2014-10-10 Thread rmcilroy
Please update the description to mention the reason why this is a problem currently. Also please add the tracking bug in a BUG= line. https://codereview.chromium.org/643163002/diff/1/tools/gyp/v8.gyp File tools/gyp/v8.gyp (right):

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-10-09 Thread rmcilroy
lgtm once comments are addressed. Ulan do you want to take a look? https://codereview.chromium.org/609843002/diff/40001/src/arm64/macro-assembler-arm64.cc File src/arm64/macro-assembler-arm64.cc (right):

[v8-dev] Remove default NOP implementation of MonotonicallyIncreasingTime. (issue 641653002 by rmcil...@chromium.org)

2014-10-09 Thread rmcilroy
{ * it is recommended that the fixed point be no further in the past than * the epoch. **/ - virtual double MonotonicallyIncreasingTime() { -// TODO(rmcilroy): Remove this default implementation when Chromium -// change has landed. -return 0; - } + virtual double

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-10-09 Thread rmcilroy
+simonb https://codereview.chromium.org/609843002/ -- -- 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 default NOP implementation of MonotonicallyIncreasingTime. (issue 641653002 by rmcil...@chromium.org)

2014-10-09 Thread rmcilroy
Committed patchset #1 (id:1) manually as 24489 (presubmit successful). https://codereview.chromium.org/641653002/ -- -- 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] Add MonotonicallyIncreasingTime to V8 Platform. (issue 632663002 by rmcil...@chromium.org)

2014-10-06 Thread rmcilroy
MonotonicallyIncreasingTime() { +// TODO(rmcilroy): Remove this default implementation when Chromium +// change has landed. +return 0; + } }; } // namespace v8 Index: src/libplatform/default-platform.cc diff --git a/src/libplatform/default-platform.cc b/src/libplatform/default

[v8-dev] Re: Add MonotonicallyIncreasingTime to V8 Platform. (issue 632663002 by rmcil...@chromium.org)

2014-10-06 Thread rmcilroy
Committed patchset #1 (id:1) manually as 24411 (presubmit successful). https://codereview.chromium.org/632663002/ -- -- 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] Re: Extend snapshot creation to generate 4-byte values (issue 623993002 by ba...@chromium.org)

2014-10-03 Thread rmcilroy
lgtm with one comment - I'll leave it for yang on Daniel to approve though. https://codereview.chromium.org/623993002/diff/1/tools/js2c.py File tools/js2c.py (right): https://codereview.chromium.org/623993002/diff/1/tools/js2c.py#newcode502 tools/js2c.py:502: size = 4 nit - please move this

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-10-02 Thread rmcilroy
https://codereview.chromium.org/609843002/diff/20001/src/arm64/assembler-arm64.h File src/arm64/assembler-arm64.h (right): https://codereview.chromium.org/609843002/diff/20001/src/arm64/assembler-arm64.h#newcode1903 src/arm64/assembler-arm64.h:1903: I think we should put both

[v8-dev] Enable out-of-line constant pool for Arm. (issue 610963003 by rmcil...@chromium.org)

2014-09-29 Thread rmcilroy
Reviewers: ulan, Message: Let's try this again shall we? Description: Enable out-of-line constant pool for Arm. Please review this at https://codereview.chromium.org/610963003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+4, -0 lines): M src/globals.h

[v8-dev] Re: Enable out-of-line constant pool for Arm. (issue 610963003 by rmcil...@chromium.org)

2014-09-29 Thread rmcilroy
Committed patchset #3 (id:40001) manually as 24288 (presubmit successful). https://codereview.chromium.org/610963003/ -- -- 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

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-09-26 Thread rmcilroy
On 2014/09/26 15:54:58, rmcilroy wrote: https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h File src/ia32/assembler-ia32.h (right): https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h#newcode1102 src/ia32/assembler-ia32.h:1102: // Support

[v8-dev] Re: Refactor FrameAndConstantPoolScope (issue 609843002 by ba...@chromium.org)

2014-09-26 Thread rmcilroy
https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h File src/ia32/assembler-ia32.h (right): https://codereview.chromium.org/609843002/diff/1/src/ia32/assembler-ia32.h#newcode1102 src/ia32/assembler-ia32.h:1102: // Support not implemented in ia32 yet. We will probably

[v8-dev] Re: ARM: Make stack limit stricter to account for large buffers in MacroAssembler. (issue 583163002 by u...@chromium.org)

2014-09-19 Thread rmcilroy
On 2014/09/19 09:40:26, ulan wrote: PTAL. This is another fix for issue 405338 and replaces https://codereview.chromium.org/555943003 Seems reasonable, thanks. lgtm. https://codereview.chromium.org/583163002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Endian changes, support 64bit big endian (issue 551803004 by andrew_...@ca.ibm.com)

2014-09-18 Thread rmcilroy
https://codereview.chromium.org/551803004/diff/1/src/objects.h File src/objects.h (right): https://codereview.chromium.org/551803004/diff/1/src/objects.h#newcode3171 src/objects.h:3171: kExtendedInt64CountOffset + kInt32Size; On 2014/09/16 10:48:40, Sven Panne wrote: I think that this is

[v8-dev] Re: ARM: Do not stack allocate big buffers in Assembler. (issue 555943003 by u...@chromium.org)

2014-09-18 Thread rmcilroy
On 2014/09/18 12:52:42, ulan wrote: Hi Ross, Rodolph. Could you please take a look? This fixes chrome crashes. Another solution would be to reduce FLAG_stack_size by 120KB for ARM. I will check benchmarks before landing. This seems fine to me, but I would just always allocate

[v8-dev] Re: ARM: Do not stack allocate big buffers in Assembler. (issue 555943003 by u...@chromium.org)

2014-09-18 Thread rmcilroy
On 2014/09/18 13:48:22, Sven Panne wrote: On 2014/09/18 13:02:03, rmcilroy wrote: This seems fine to me, but I would just always allocate kMaxNumPending32/64RelocInfo if you are wanting to be cautious (for merge into m38). If malloc is smart enough to keep the allocation in a size-binned

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

2014-08-29 Thread rmcilroy
On 2014/08/29 13:59:07, baixo wrote: Could you guys take another look, please? Builds were not working correctly because the blobs could be generated from multiple targets (host and target). This was fixed. lgtm, I'll commit if vogelheim thinks this is OK.

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

2014-08-29 Thread rmcilroy
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', On 2014/08/29 14:10:55, vogelheim wrote: I'm a bit

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

2014-08-29 Thread rmcilroy
Committed patchset #2 (id:20001) manually as 23522 (presubmit successful). 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

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

2014-08-29 Thread rmcilroy
Reviewers: vogelheim, Message: Messed up the patch, sorry. Description: Fix missing bracket in gyp file from r23522. TBR=vogelh...@chromium.org Please review this at https://codereview.chromium.org/521613003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+1,

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

2014-08-29 Thread rmcilroy
Committed patchset #1 (id:1) manually as 23523 (presubmit successful). 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.

[v8-dev] Re: Revert Enable out-of-line constant pool for Arm. (issue 502723002 by machenb...@chromium.org)

2014-08-28 Thread rmcilroy
On 2014/08/28 08:30:11, ulan wrote: On 2014/08/27 17:54:51, Michael Achenbach wrote: On 2014/08/26 10:49:38, rmcilroy wrote: On 2014/08/23 18:38:48, Michael Achenbach wrote: Committed patchset #1 manually as 23325 (tree was closed). Thanks for reverting (apologies for not spotting

[v8-dev] Re: MIPS: Fix [de]serialize problem of root objects. (issue 492303004 by balazs.kilv...@imgtec.com)

2014-08-28 Thread rmcilroy
On 2014/08/28 10:38:15, Yang wrote: On 2014/08/28 09:54:43, balazs.kilvady wrote: On 2014/08/28 09:48:54, Yang wrote: On 2014/08/28 09:45:19, balazs.kilvady wrote: PTAL. Could a v8 team member review? Why is it necessary for MIPS? Would the same also apply for ARM? Thank you for the

[v8-dev] Re: Revert Enable out-of-line constant pool for Arm. (issue 502723002 by machenb...@chromium.org)

2014-08-26 Thread rmcilroy
On 2014/08/23 18:38:48, Michael Achenbach wrote: Committed patchset #1 manually as 23325 (tree was closed). Thanks for reverting (apologies for not spotting this, the bot took a while to go red and it was after I'd gone home). I've just tried running the command on the bot which timedout

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

2014-08-26 Thread rmcilroy
Committed patchset #3 manually as 23416 (presubmit successful). 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

<    1   2   3   4   5   6   7   8   >