[v8-dev] Re: Fix stale pointer issue in heap snapshot generator (issue 1109153002 by jkumme...@chromium.org)

2015-04-29 Thread loislo
On 2015/04/29 09:03:23, I haz the power (commit-bot) wrote: Patchset 1 (id:??) landed as https://crrev.com/fefe91ce6a49b644e909faa92bee10814eaed729 Cr-Commit-Position: refs/heads/master@{#28124} lgtm https://codereview.chromium.org/1109153002/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Fix C++ violation. (issue 1073903002 by tha...@chromium.org)

2015-04-09 Thread loislo
https://codereview.chromium.org/1073903002/diff/1/include/v8-profiler.h File include/v8-profiler.h (left): https://codereview.chromium.org/1073903002/diff/1/include/v8-profiler.h#oldcode36 include/v8-profiler.h:36: std::vector stack; template class needs to be defined before the first usage. Oth

[v8-dev] Re: CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-31 Thread loislo
On 2015/03/31 at 08:23:34, commit-bot wrote: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1045753002/130001 I started CQ dry run https://codereview.chromium.org/1045753002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.

[v8-dev] Re: CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-31 Thread loislo
https://codereview.chromium.org/1045753002/diff/90001/include/v8-profiler.h File include/v8-profiler.h (right): https://codereview.chromium.org/1045753002/diff/90001/include/v8-profiler.h#newcode25 include/v8-profiler.h:25: unsigned int position; On 2015/03/31 at 06:53:49, yurys wrote: On 2015/

[v8-dev] Re: CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-31 Thread loislo
On 2015/03/31 at 06:53:49, yurys wrote: https://codereview.chromium.org/1045753002/diff/90001/include/v8-profiler.h File include/v8-profiler.h (right): https://codereview.chromium.org/1045753002/diff/90001/include/v8-profiler.h#newcode25 include/v8-profiler.h:25: unsigned int position; On 201

[v8-dev] Re: CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-30 Thread loislo
PTAL https://codereview.chromium.org/1045753002/ -- -- 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 i

[v8-dev] Re: CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-30 Thread loislo
comments addressed https://codereview.chromium.org/1045753002/diff/50001/include/v8-profiler.h File include/v8-profiler.h (right): https://codereview.chromium.org/1045753002/diff/50001/include/v8-profiler.h#newcode102 include/v8-profiler.h:102: /** Retrieves a child node by index. */ On 2015/0

[v8-dev] CpuProfiler: public API for deopt info in cpu profiler. (issue 1045753002 by loi...@chromium.org)

2015-03-30 Thread loislo
Reviewers: yurys, alph, Message: PTAL Description: CpuProfiler: public API for deopt info in cpu profiler. BUG=chromium:452067 LOG=n Please review this at https://codereview.chromium.org/1045753002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+62, -9 lines):

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-24 Thread loislo
On 2015/03/24 at 12:09:18, loislo wrote: On 2015/03/24 at 10:56:55, svenpanne wrote: > https://codereview.chromium.org/1013143003/diff/190001/src/compiler.h > File src/compiler.h (right): > > https://codereview.chromium.org/1013143003/diff/190001/src/compiler.h#newcode352 > src

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-24 Thread loislo
nfos() { On 2015/03/24 10:46:34, loislo wrote: > 1) We pass CompilationInfo to logger when we finished compilation, so it is > alive. > > 2) Profiler copies the inlining info into CodeCreateEvent (it used to take the > ownership in the previous versions of the patch). To be exa

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-24 Thread loislo
https://codereview.chromium.org/1013143003/diff/190001/src/compiler.h File src/compiler.h (right): https://codereview.chromium.org/1013143003/diff/190001/src/compiler.h#newcode352 src/compiler.h:352: const std::vector& inlined_function_infos() { On 2015/03/24 at 10:25:30, Sven Panne wrote: Hmmm

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-24 Thread loislo
On 2015/03/23 at 13:01:47, loislo wrote: On 2015/03/23 at 10:13:27, svenpanne wrote: > I'm a bit unhappy about the complicated ownership/lifetime handling in this CL. It looks like premature optimization in an already horribly complicated and bloated class (CompilationInfo). Unless

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-23 Thread loislo
On 2015/03/23 at 10:13:27, svenpanne wrote: I'm a bit unhappy about the complicated ownership/lifetime handling in this CL. It looks like premature optimization in an already horribly complicated and bloated class (CompilationInfo). Unless we have hard measured numbers to justify this, we sh

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-20 Thread loislo
https://codereview.chromium.org/1013143003/ -- -- 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, sen

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-20 Thread loislo
comments addressed https://codereview.chromium.org/1013143003/diff/130001/src/compiler.h File src/compiler.h (right): https://codereview.chromium.org/1013143003/diff/130001/src/compiler.h#newcode468 src/compiler.h:468: std::vector* inlined_function_infos_; On 2015/03/20 at 08:28:42, alph wrote

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-19 Thread loislo
On 2015/03/19 at 15:01:31, loislo wrote: On 2015/03/19 at 15:01:16, loislo wrote: > +svenpanne@ the patch needs a rebaseline rebaselined. PTAL https://codereview.chromium.org/1013143003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev ---

[v8-dev] Re: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-19 Thread loislo
On 2015/03/19 at 15:01:16, loislo wrote: +svenpanne@ the patch needs a rebaseline https://codereview.chromium.org/1013143003/ -- -- 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: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-19 Thread loislo
+svenpanne@ https://codereview.chromium.org/1013143003/ -- -- 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: CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-19 Thread loislo
comments addressed https://codereview.chromium.org/1013143003/diff/30001/src/profile-generator.cc File src/profile-generator.cc (right): https://codereview.chromium.org/1013143003/diff/30001/src/profile-generator.cc#newcode123 src/profile-generator.cc:123: info.AddInlineFrame(script_id_, posit

[v8-dev] CpuProfiler: push the collected information about deopts to cpu profiler (issue 1013143003 by loi...@chromium.org)

2015-03-18 Thread loislo
Reviewers: alph, yurys, Message: ptal Description: CpuProfiler: push the collected information about deopts to cpu profiler it is the last patch of https://codereview.chromium.org/1012633002 All that we need here is to push the collected info to the profiler and convert it into actionable info

[v8-dev] CodeCleanup: eliminate unnecessary base class and make the children unvirtual. (issue 1010413003 by loi...@chromium.org)

2015-03-18 Thread loislo
Reviewers: jarin, Rico, Message: PTAL Description: CodeCleanup: eliminate unnecessary base class and make the children unvirtual. I found some strange split in deopt entry points generator. The code for table entry generator had two classes. It is safe to join these classes together and drop

[v8-dev] CpuProfiler: log pc offset for deopts. (issue 1011113004 by loi...@chromium.org)

2015-03-18 Thread loislo
Reviewers: alph, yurys, Sven Panne, Message: PTAL Description: CpuProfiler: log pc offset for deopts. This is the fifth part of https://codereview.chromium.org/1012633002 In this part we collect the offsets of deopt calls and save it into an inlined function info. On the Next: Later when deopt

[v8-dev] Re: CpuProfiler: x87. put right address to the stack, so the callee would be able to resolve it into th… (issue 1013243002 by loi...@chromium.org)

2015-03-17 Thread loislo
On 2015/03/17 at 18:01:58, alph wrote: answer the comment and then lgtm https://codereview.chromium.org/1013243002/diff/1/src/x87/lithium-codegen-x87.cc File src/x87/lithium-codegen-x87.cc (right): https://codereview.chromium.org/1013243002/diff/1/src/x87/lithium-codegen-x87.cc#newcode392

[v8-dev] CpuProfiler: x87. put right address to the stack, so the callee would be able to resolve it into th… (issue 1013243002 by loi...@chromium.org)

2015-03-17 Thread loislo
Reviewers: jarin, Sven Panne, alph, Message: PTAL Description: CpuProfiler: x87. put right address to the stack, so the callee would be able to resolve it into the right deopt_info. 'from' is using for Code object lookup and will be used for inline_id lookup. see https://codereview.chromium.

[v8-dev] Re: CpuProfiler: collect deopt pc offset for further usage in the inlined functions stack resolver. (issue 1013753007 by loi...@chromium.org)

2015-03-17 Thread loislo
comments addressed https://codereview.chromium.org/1013753007/diff/1/src/cpu-profiler.h File src/cpu-profiler.h (right): https://codereview.chromium.org/1013753007/diff/1/src/cpu-profiler.h#newcode83 src/cpu-profiler.h:83: int pc_offset; On 2015/03/17 at 15:24:05, Sven Panne wrote: I would re

[v8-dev] Re: CpuProfiler: ia32. put right address to the stack, so the callee would be able to resolve it into t… (issue 1014783002 by loi...@chromium.org)

2015-03-17 Thread loislo
On 2015/03/17 at 14:54:09, svenpanne wrote: On 2015/03/17 14:48:39, loislo wrote: > Does Red Zone helps in this case? > http://en.wikipedia.org/wiki/Red_zone_(computing) IIRC, this is an ABI issue: Windows, Xen, HaLVM, and probably a few others don't impose a mandatory red zone (a

[v8-dev] Re: CpuProfiler: ia32. put right address to the stack, so the callee would be able to resolve it into t… (issue 1014783002 by loi...@chromium.org)

2015-03-17 Thread loislo
On 2015/03/17 at 12:44:22, jarin wrote: https://codereview.chromium.org/1014783002/diff/20001/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): https://codereview.chromium.org/1014783002/diff/20001/src/ia32/lithium-codegen-ia32.cc#newcode394 src/ia32/lithium-cod

[v8-dev] Re: CpuProfiler: collect deopt pc offset for further usage in the inlined functions stack resolver. (issue 1013753007 by loi...@chromium.org)

2015-03-17 Thread loislo
ptal https://codereview.chromium.org/1013753007/ -- -- 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 i

[v8-dev] CpuProfiler: collect deopt pc offset for further usage in the inlined functions stack resolver. (issue 1013753007 by loi...@chromium.org)

2015-03-17 Thread loislo
Reviewers: yurys, alph, Message: PTAL Description: CpuProfiler: collect deopt pc offset for further usage in the inlined functions stack resolver. this is a forth part of https://codereview.chromium.org/1012633002 In another patch I'll collect the inlining tree in cpu-profiler CodeEntry Eac

[v8-dev] CpuProfiler: ia32. put right address to the stack, so the callee would be able to resolve it into t… (issue 1014783002 by loi...@chromium.org)

2015-03-17 Thread loislo
Reviewers: Sven Panne, jarin, alph, Message: PTAL Description: CpuProfiler: ia32. put right address to the stack, so the callee would be able to resolve it into the right deopt_info. 'from' is using for Code object lookup and will be used for inline_id lookup. see https://codereview.chromium

[v8-dev] Re: CpuProfiler: extract DeoptInfo fill in code into a static function. (issue 1011733005 by loi...@chromium.org)

2015-03-17 Thread loislo
+jarin@ https://codereview.chromium.org/1011733005/ -- -- 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 fro

[v8-dev] CpuProfiler: extract DeoptInfo fill in code into a static function. (issue 1011733005 by loi...@chromium.org)

2015-03-16 Thread loislo
Reviewers: Sven Panne, alph, yurys, Message: PTAL Description: CpuProfiler: extract DeoptInfo fill in code into a static function. the third part of the patch https://codereview.chromium.org/1012633002 this patch 1) moves DeoptInfo builder code to platform independent file lithium-codegen.cc

[v8-dev] CpuProfiler: put right address to the stack, so the callee would be able to resolve it into the rig… (issue 1012743002 by loi...@chromium.org)

2015-03-16 Thread loislo
Reviewers: Sven Panne, alph, yurys, Message: PTAL Description: CpuProfiler: put right address to the stack, so the callee would be able to resolve it into the right deopt_info. 'from' is using for Code object lookup and will be used for inline_id lookup. see https://codereview.chromium.org/1012

[v8-dev] Re: CpuProfiler: replace FLAG_hydrogen_track_positions with is_tracking_positions method on Compilation… (issue 995183005 by loi...@chromium.org)

2015-03-16 Thread loislo
comments addressed https://codereview.chromium.org/995183005/diff/1/src/compiler.cc File src/compiler.cc (right): https://codereview.chromium.org/995183005/diff/1/src/compiler.cc#newcode291 src/compiler.cc:291: if (FLAG_hydrogen_track_positions) { On 2015/03/16 at 13:16:11, yurys wrote: && !sc

[v8-dev] CpuProfiler: replace FLAG_hydrogen_track_positions with is_tracking_positions method on Compilation… (issue 995183005 by loi...@chromium.org)

2015-03-16 Thread loislo
Reviewers: Sven Panne, yurys, alph, Message: PTAL Description: CpuProfiler: replace FLAG_hydrogen_track_positions with is_tracking_positions method on CompilationInfo this is the second part of https://codereview.chromium.org/1012633002. almost mechanical change. I'd like to enable position

[v8-dev] CpuProfiler: convert List into std::vector (issue 1011733002 by loi...@chromium.org)

2015-03-16 Thread loislo
Reviewers: Sven Panne, yurys, Message: this is a part of https://codereview.chromium.org/1012633002 PTAL Description: CpuProfiler: convert List into std::vector mechanical change. BUG=chromium:452067 LOG=n Please review this at https://codereview.chromium.org/1011733002/ Base URL: https://c

[v8-dev] CpuProfiler: Push inlining data forward to cpu profiler. (issue 1012633002 by loi...@chromium.org)

2015-03-16 Thread loislo
Reviewers: Sven Panne, yurys, alph, Description: CpuProfiler: Push the inlining data forward to cpu profiler. The schema: 0) make inlining id accessible via LEnvironment->entry 1) push the map between pc_offset and inlining id into CodeEntry 2) collect inlining tree (script_ids, start_position

[v8-dev] Re: CpuProfiler: Push inlining data forward to cpu profiler. (issue 1012633002 by loi...@chromium.org)

2015-03-16 Thread loislo
PTAL https://codereview.chromium.org/1012633002/ -- -- 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 i

[v8-dev] Re: CpuProfiler: do not calculate positions if it is not necessary (first part). (issue 961283002 by loi...@chromium.org)

2015-03-12 Thread loislo
On 2015/03/12 11:44:59, Sven Panne wrote: What's the status of this CL? Is it obsolete? What about Slava's comment? it needs to be reworked. I'll do this in another patch. https://codereview.chromium.org/961283002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/gro

[v8-dev] Re: MIPS64: CpuProfiler: fix for CollectDeoptEvents test. (issue 993233003 by balazs.kilv...@imgtec.com)

2015-03-11 Thread loislo
lgtm https://codereview.chromium.org/993233003/ -- -- 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

[v8-dev] Re: CpuProfiler: simplify inlined function info magic. (issue 996153003 by loi...@chromium.org)

2015-03-11 Thread loislo
https://codereview.chromium.org/996153003/diff/1/src/compiler.cc File src/compiler.cc (right): https://codereview.chromium.org/996153003/diff/1/src/compiler.cc#newcode278 src/compiler.cc:278: int pareint_id) { On 2015/03/11 13:05:21, yurys wrote: typo: parent_id Done. https://codereview.chro

[v8-dev] CpuProfiler: simplify inlined function info magic. (issue 996153003 by loi...@chromium.org)

2015-03-11 Thread loislo
Reviewers: yurys, alph, Sven Panne, Message: PTAL Description: CpuProfiler: simplify inlined function info magic. I did some investigation and found that in the most cases the old schema with the separate List for functions and inlines gives us no memory benefits because more frequently we inli

[v8-dev] Re: MIPS: CpuProfiler: fix for CollectDeoptEvents test. (issue 996883002 by balazs.kilv...@imgtec.com)

2015-03-11 Thread loislo
-codegen-mips.cc:365: __ push(at); On 2015/03/10 19:32:41, loislo wrote: > Would it be more effective to move ra from upper MultiPush to here? ra must be saved before the Call() as Call() overrides ra. The upper MultiPush() is the exact port of arm PushFixedFrame(). https://codereview.chromium.

[v8-dev] Re: MIPS: CpuProfiler: fix for CollectDeoptEvents test. (issue 996883002 by balazs.kilv...@imgtec.com)

2015-03-10 Thread loislo
https://codereview.chromium.org/996883002/diff/1/src/mips/lithium-codegen-mips.cc File src/mips/lithium-codegen-mips.cc (right): https://codereview.chromium.org/996883002/diff/1/src/mips/lithium-codegen-mips.cc#newcode365 src/mips/lithium-codegen-mips.cc:365: __ push(at); Would it be more effect

[v8-dev] Re: CpuProfiler: fix for CollectDeoptEvents test on arm64 (issue 984893003 by loi...@chromium.org)

2015-03-10 Thread loislo
https://codereview.chromium.org/984893003/diff/20001/src/arm64/lithium-codegen-arm64.cc File src/arm64/lithium-codegen-arm64.cc (right): https://codereview.chromium.org/984893003/diff/20001/src/arm64/lithium-codegen-arm64.cc#newcode870 src/arm64/lithium-codegen-arm64.cc:870: __ Push(lr, fp, cp);

[v8-dev] Re: CpuProfiler: fix for CollectDeoptEvents test on arm (issue 997513003 by loi...@chromium.org)

2015-03-10 Thread loislo
https://codereview.chromium.org/997513003/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://codereview.chromium.org/997513003/diff/1/src/arm/lithium-codegen-arm.cc#newcode356 src/arm/lithium-codegen-arm.cc:356: bool need_branch = ((i + 1) != length); On 2

[v8-dev] CpuProfiler: slightly reduce the size of JumpTable code. (issue 995813002 by loi...@chromium.org)

2015-03-10 Thread loislo
Reviewers: jbramley, Sven Panne, yurys, Message: PTAL Description: CpuProfiler: slightly reduce the size of JumpTable code. BUG=chromium:452067 LOG=n R=svenpa...@chromium.org, jacob.bram...@arm.com, yu...@chromium.org Please review this at https://codereview.chromium.org/995813002/ Base URL:

[v8-dev] CpuProfiler: fix for CollectDeoptEvents test on arm (issue 997513003 by loi...@chromium.org)

2015-03-10 Thread loislo
Reviewers: Sven Panne, yurys, alph, jbramley, Message: PTAL Description: CpuProfiler: fix for CollectDeoptEvents test on arm The same idea as it was in https://codereview.chromium.org/984893003/ BUG=chromium:452067 LOG=n Please review this at https://codereview.chromium.org/997513003/ Base U

[v8-dev] Re: Remove deprecated CpuProfiler methods (issue 992193002 by yu...@chromium.org)

2015-03-10 Thread loislo
lgtm https://codereview.chromium.org/992193002/ -- -- 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] CpuProfiler: fix for CollectDeoptEvents test on arm64 (issue 984893003 by loi...@chromium.org)

2015-03-09 Thread loislo
Reviewers: Sven Panne, yurys, alph, Message: PTAL Description: CpuProfiler: fix for CollectDeoptEvents test on arm64 We use slightly different schema for JumpTable on arm64 than for x64. We do a branch (B) to the jump table from the code, then a branch (B) to the end of jump table and then bra

[v8-dev] Re: CpuProfiler: fix for GetDeoptReason code. (issue 984773003 by loi...@chromium.org)

2015-03-09 Thread loislo
https://codereview.chromium.org/984773003/diff/20001/src/cpu-profiler.cc File src/cpu-profiler.cc (right): https://codereview.chromium.org/984773003/diff/20001/src/cpu-profiler.cc#newcode332 src/cpu-profiler.cc:332: void CpuProfiler::CodeDeoptEvent(Code* code, int bailout_id, Address pc, On 2015

[v8-dev] Re: CpuProfiler: fix for GetDeoptReason code. (issue 984773003 by loi...@chromium.org)

2015-03-06 Thread loislo
PTAL https://codereview.chromium.org/984773003/ -- -- 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

[v8-dev] CpuProfiler: fix for GetDeoptReason code. (issue 984773003 by loi...@chromium.org)

2015-03-06 Thread loislo
Reviewers: alph, yurys, Message: PTAL Description: CpuProfiler: fix for GetDeoptReason code. The original code always returned the first entry from RelocInfo that matched with bailout_id. But we may have a few different deopt reasons for one bailout_id. So we need to get the one which matc

[v8-dev] Re: CpuProfiler: enable tests except four failing tests. (issue 976203003 by loi...@chromium.org)

2015-03-06 Thread loislo
A revert of this CL (patchset #3 id:11) has been created in https://codereview.chromium.org/987553005/ by loi...@chromium.org. The reason for reverting is: Some tests still flaky. https://codereview.chromium.org/976203003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.goog

[v8-dev] Revert of CpuProfiler: enable tests except four failing tests. (issue 987553005 by loi...@chromium.org)

2015-03-06 Thread loislo
Reviewers: yurys, Sven Panne, Message: Created Revert of CpuProfiler: enable tests except four failing tests. Description: Revert of CpuProfiler: enable tests except four failing tests. (patchset #3 id:11 of https://codereview.chromium.org/976203003/) Reason for revert: Some tests still fla

[v8-dev] Re: CpuProfiler: enable tests except four failing tests. (issue 976203003 by loi...@chromium.org)

2015-03-05 Thread loislo
On 2015/03/06 06:38:43, loislo wrote: On 2015/03/05 15:05:48, Sven Panne wrote: > Good point, I didn't see that.. > > https://codereview.chromium.org/976203003/diff/20001/test/cctest/test-cpu-profiler.cc > File test/cctest/test-cpu-profiler.cc (right): > > https://

[v8-dev] Re: CpuProfiler: enable tests except four failing tests. (issue 976203003 by loi...@chromium.org)

2015-03-05 Thread loislo
On 2015/03/05 15:05:48, Sven Panne wrote: Good point, I didn't see that.. https://codereview.chromium.org/976203003/diff/20001/test/cctest/test-cpu-profiler.cc File test/cctest/test-cpu-profiler.cc (right): https://codereview.chromium.org/976203003/diff/20001/test/cctest/test-cpu-profiler.

[v8-dev] CpuProfiler: enable tests except four failing tests. (issue 976203003 by loi...@chromium.org)

2015-03-05 Thread loislo
Reviewers: yurys, Sven Panne, Message: PTAL Description: CpuProfiler: enable tests except four failing tests. Four tests are failing due to a problem with no frame ranges. BUG= LOG=n Please review this at https://codereview.chromium.org/976203003/ Base URL: https://chromium.googlesource.com/

[v8-dev] Re: CpuProfiler: simplify test. (issue 978203002 by loi...@chromium.org)

2015-03-05 Thread loislo
Reviewers: svenpanne, yurys, https://codereview.chromium.org/978203002/diff/1/test/cctest/test-cpu-profiler.cc File test/cctest/test-cpu-profiler.cc (right): https://codereview.chromium.org/978203002/diff/1/test/cctest/test-cpu-profiler.cc#newcode59 test/cctest/test-cpu-profiler.cc:59: static c

[v8-dev] CpuProfiler: Push inlining data forward to cpu profiler. (issue 972703003 by loi...@chromium.org)

2015-03-05 Thread loislo
Reviewers: alph, yurys, Message: PTAL Description: CpuProfiler: Push inlining data forward to cpu profiler. The schema: 1) collect inlining tree (script_ids, start_positions and inlining_positions) and put it into CodeEntry 2) In order of processing CodeDeoptEvent take the branch from the i

[v8-dev] Re: Fix for mips64 after #26916 (issue 960903005 by loi...@chromium.org)

2015-02-27 Thread loislo
Reviewers: Sven Panne, yurys, Description: Fix for mips64 after #26916 BUG= TBR=svenpanne Please review this at https://codereview.chromium.org/960903005/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+1, -0 lines): M src/mips64/assembler-mips64.h Index: src

[v8-dev] CpuProfiler: do not calculate positions if it is not necessary (TryInline part). (issue 962593005 by loi...@chromium.org)

2015-02-27 Thread loislo
Reviewers: yurys, alph, svenpanne, Message: PTAL Description: CpuProfiler: do not calculate positions if it is not necessary (TryInline part). TryInline needed position only for the case when we track positions. We can drop the position argument and use the current position from GraphBuilder

[v8-dev] CpuProfiler: do not calculate positions if it is not necessary (first part). (issue 961283002 by loi...@chromium.org)

2015-02-27 Thread loislo
Reviewers: yurys, alph, Sven Panne, Message: PTAL Description: CpuProfiler: do not calculate positions if it is not necessary (first part). BUG=452067 LOG=n Please review this at https://codereview.chromium.org/961283002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected

[v8-dev] Re: CpuProfiler: replace raw position wit SourcePosition for DeoptReason (issue 959203002 by loi...@chromium.org)

2015-02-27 Thread loislo
comments addressed https://codereview.chromium.org/959203002/diff/20001/src/assembler.cc File src/assembler.cc (right): https://codereview.chromium.org/959203002/diff/20001/src/assembler.cc#newcode1663 src/assembler.cc:1663: int raw_position = position.IsUnknown() ? 0 : position.raw(); On 2015

[v8-dev] CpuProfiler: replace raw position wit SourcePosition for DeoptReason (issue 959203002 by loi...@chromium.org)

2015-02-27 Thread loislo
Reviewers: alph, yurys, Sven Panne, Message: PTAL Description: CpuProfiler: replace raw position with SourcePosition for DeoptReason Save Unknown position as zero in RelocInfo. Remove copy constructor of SourcePosition because it is trivial. Mechanical replace int raw_position with SourcePosit

[v8-dev] Re: CpuProfiler: move StringsStorage class to separate source and header files. (issue 945873002 by loi...@chromium.org)

2015-02-20 Thread loislo
On 2015/02/20 14:07:17, yurys wrote: lgtm, I believe we don't need StringStorage for CPU profiler and could eliminate its usages there. I'll try to do it in another patch later. https://codereview.chromium.org/945873002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.goo

[v8-dev] Re: CpuProfiler: move StringsStorage class to separate source and header files. (issue 945873002 by loi...@chromium.org)

2015-02-20 Thread loislo
On 2015/02/20 13:50:27, loislo wrote: PTAL https://codereview.chromium.org/945873002/ -- -- 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] CpuProfiler: move StringsStorage class to separate source and header files. (issue 945873002 by loi...@chromium.org)

2015-02-20 Thread loislo
Reviewers: yurys, alph, svenpanne, Description: CpuProfiler: move StringsStorage class to separate source and header files. Mechanical change. This will break dependency between profiler-generator and heap-profiler-generator. Later this will help us to reuse SourcePosition in cpu-profiler. BUG

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
On 2015/02/20 12:34:25, yurys wrote: Also please update issue's description as its main purpose is to make CallUid actually unique and eliminate SharedFunctionInfo motion events. done https://codereview.chromium.org/941973002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
On 2015/02/20 12:29:29, yurys wrote: https://codereview.chromium.org/941973002/diff/11/src/profile-generator.cc File src/profile-generator.cc (right): https://codereview.chromium.org/941973002/diff/11/src/profile-generator.cc#newcode351 src/profile-generator.cc:351: function_ids_.Look

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
On 2015/02/20 12:25:31, alph wrote: lgtm https://codereview.chromium.org/941973002/diff/11/src/profile-generator.cc File src/profile-generator.cc (right): https://codereview.chromium.org/941973002/diff/11/src/profile-generator.cc#newcode198 src/profile-generator.cc:198: if (script_i

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
https://codereview.chromium.org/941973002/diff/60001/src/profile-generator.cc File src/profile-generator.cc (right): https://codereview.chromium.org/941973002/diff/60001/src/profile-generator.cc#newcode198 src/profile-generator.cc:198: (script_id_ != v8::UnboundScript::kNoScriptId || On 2015/02/

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
https://codereview.chromium.org/941973002/diff/20001/src/cpu-profiler.h File src/cpu-profiler.h (right): https://codereview.chromium.org/941973002/diff/20001/src/cpu-profiler.h#newcode243 src/cpu-profiler.h:243: virtual void SharedFunctionInfoMoveEvent(Address from, Address to) {} On 2015/02/20

[v8-dev] Re: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
https://codereview.chromium.org/941973002/diff/20001/src/profile-generator.cc File src/profile-generator.cc (left): https://codereview.chromium.org/941973002/diff/20001/src/profile-generator.cc#oldcode175 src/profile-generator.cc:175: hash ^= ComputeIntegerHash(static_cast(shared_id_), On 2015/0

[v8-dev] CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h (issue 941973002 by loi...@chromium.org)

2015-02-20 Thread loislo
Reviewers: alph, yurys, Sven Panne, Message: PTAL Description: CpuProfiler: eliminate cpu-profiler dependency from heap-inl.h and replace manually generated shared_id with script_id+position BUG=452067 LOG=n Please review this at https://codereview.chromium.org/941973002/ Base URL: https://c

[v8-dev] Re: A few tests fails when I run them with --hydrogen-track-positions (issue 940593002 by loi...@chromium.org)

2015-02-19 Thread loislo
done https://codereview.chromium.org/940593002/ -- -- 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 i

[v8-dev] Re: A few tests fails when I run them with --hydrogen-track-positions (issue 940593002 by loi...@chromium.org)

2015-02-18 Thread loislo
On 2015/02/18 18:10:56, loislo wrote: PTAL the stack trace # # Fatal error in .././src/utils.h, line 220 # Check failed: is_valid(value). # C stack trace === 1: V8_Fatal 2: v8::internal::BitFieldBase::encode(unsigned int) 3: v8::internal::BitFieldBase

[v8-dev] A few tests fails when I run them with --hydrogen-track-positions (issue 940593002 by loi...@chromium.org)

2015-02-18 Thread loislo
Reviewers: Vyacheslav Egorov, Sven Panne, yurys, Message: PTAL Description: A few tests fails when I run them with --hydrogen-track-positions BUG=452067 TEST=test-cpu-profiler/SourceLocation LOG=n Please review this at https://codereview.chromium.org/940593002/ Base URL: https://chromium.goog

[v8-dev] Adjust types in SourcePosition. int -> uint32_t (issue 931163002 by loi...@chromium.org)

2015-02-17 Thread loislo
Reviewers: Sven Panne, yurys, alph, Message: PTAL Description: Adjust types in SourcePosition. int -> uint32_t BUG= LOG=n Please review this at https://codereview.chromium.org/931163002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+16, -16 lines): M src/com

[v8-dev] CpuProfile: rename HSourcePosition to SourcePosition and move it to compiler.* (issue 928343003 by loi...@chromium.org)

2015-02-17 Thread loislo
Reviewers: Sven Panne, yurys, alph, Message: PTAL It is almost mechanical patch Description: CpuProfile: rename HSourcePosition to SourcePosition and move it to compiler.* Fix CompilationInfo::TraceInlinedFunction argument. Fix leaked CodeTracer in Isolate BUG=452067 Please review this at

[v8-dev] Re: CpuProfiler: move InlinedFunctionInfo class from HGraphBuilder to CompilationInfo. (issue 914413007 by loi...@chromium.org)

2015-02-16 Thread loislo
I saw IRHydra2 page. The idea is the same but there is no UI yet. The only difference is that DevTools has full sources, probably with source maps so I need to push absolute offset to the cpu profile. https://codereview.chromium.org/914413007/diff/1/src/compiler.cc File src/compiler.cc (right):

[v8-dev] CpuProfiler: move InlinedFunctionInfo class from HGraphBuilder to CompilationInfo. (issue 914413007 by loi...@chromium.org)

2015-02-16 Thread loislo
Reviewers: Sven Panne, yurys, alph, Vyacheslav Egorov, Message: PTAL Description: CpuProfiler: move InlinedFunctionInfo class from HGraphBuilder to CompilationInfo. A function could be deoptimized due to a deopt in the inlined code. The inlined function might be defined in another script. So we

[v8-dev] Re: Fix compilation for the case dcheck_always_on=1. (issue 920993003 by loi...@chromium.org)

2015-02-13 Thread loislo
comments addressed. PTAL https://codereview.chromium.org/920993003/ -- -- 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 rec

[v8-dev] Fix compilation for the case dcheck_always_on=1. (issue 920993003 by loi...@chromium.org)

2015-02-13 Thread loislo
Reviewers: Toon Verwaest, Igor Sheludko, Sven Panne, Description: Fix compilation for the case dcheck_always_on=1. It is a default option for some try bots. BUG=none LOG=n Please review this at https://codereview.chromium.org/920993003/ Base URL: https://chromium.googlesource.com/v8/v8.git@ma

[v8-dev] Move identical code from platform specific assemblers to assembler.cc (issue 922153002 by loi...@chromium.org)

2015-02-12 Thread loislo
Reviewers: Sven Panne, yurys, Description: Move identical code from platform specific assemblers to assembler.cc BUG=none LOG=n Please review this at https://codereview.chromium.org/922153002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+38, -255 lines): M s

[v8-dev] Re: CPUProfiler: Push deopt reason further to ProfileNode. (issue 919953002 by loi...@chromium.org)

2015-02-12 Thread loislo
https://codereview.chromium.org/919953002/diff/1/src/profile-generator.cc File src/profile-generator.cc (right): https://codereview.chromium.org/919953002/diff/1/src/profile-generator.cc#newcode242 src/profile-generator.cc:242: if (entry->has_deopt_info()) node->CollectDeoptInfo(entry); On 2015/

[v8-dev] Revert of CPUProfiler: Push deopt reason further to ProfileNode. (issue 915173005 by loi...@chromium.org)

2015-02-12 Thread loislo
Reviewers: jarin, Sven Panne, yurys, alph, Message: Created Revert of CPUProfiler: Push deopt reason further to ProfileNode. Description: Revert of CPUProfiler: Push deopt reason further to ProfileNode. (patchset #1 id:1 of https://codereview.chromium.org/919953002/) Reason for revert: stati

[v8-dev] Re: CPUProfiler: Push deopt reason further to ProfileNode. (issue 919953002 by loi...@chromium.org)

2015-02-12 Thread loislo
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/915173005/ by loi...@chromium.org. The reason for reverting is: static initializers broke the build. https://codereview.chromium.org/919953002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://gro

[v8-dev] Re: CPUProfile: simplify test-cpu-profiler.cc with help of auto keyword (issue 925453002 by loi...@chromium.org)

2015-02-12 Thread loislo
On 2015/02/12 13:30:45, loislo wrote: On 2015/02/12 13:22:35, Sven Panne wrote: > Not LGTM: There was some more or less heated discussion here in MUC about the > use of auto, and IIRC the outcome was to restrict its use more or less to > range-based for-loops and some rare cases

[v8-dev] Re: CPUProfile: simplify test-cpu-profiler.cc with help of auto keyword (issue 925453002 by loi...@chromium.org)

2015-02-12 Thread loislo
On 2015/02/12 13:22:35, Sven Panne wrote: Not LGTM: There was some more or less heated discussion here in MUC about the use of auto, and IIRC the outcome was to restrict its use more or less to range-based for-loops and some rare cases where you would otherwise repeat a looong type. @Michi:

[v8-dev] CPUProfile: simplify test-cpu-profiler.cc with help of auto keyword (issue 925453002 by loi...@chromium.org)

2015-02-12 Thread loislo
Reviewers: yurys, alph, Sven Panne, Message: PTAL Description: CPUProfile: simplify test-cpu-profiler.cc with help of auto keyword BUG=none LOG=n Please review this at https://codereview.chromium.org/925453002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+267

[v8-dev] Re: CPUProfiler: Push deopt reason further to ProfileNode. (issue 919953002 by loi...@chromium.org)

2015-02-12 Thread loislo
https://codereview.chromium.org/919953002/diff/1/src/deoptimizer.h File src/deoptimizer.h (left): https://codereview.chromium.org/919953002/diff/1/src/deoptimizer.h#oldcode218 src/deoptimizer.h:218: (!FLAG_trace_deopt || deopt_info == other.deopt_info); On 2015/02/12 08:42:31, Sven Panne wrote:

[v8-dev] CPUProfiler: Push deopt reason further to ProfileNode. (issue 919953002 by loi...@chromium.org)

2015-02-11 Thread loislo
Reviewers: jarin, Sven Panne, yurys, alph, Message: PTAL Description: CPUProfiler: Push deopt reason further to ProfileNode. 1) create beefy RelocInfo table when cpu profiler is active, so if a function was optimized when profiler was active RelocInfo would get separate DeoptInfo for the e

[v8-dev] Re: Fix for arm64 after v8:r26448 (issue 917823002 by loi...@chromium.org)

2015-02-11 Thread loislo
Reviewers: dcarney, svenpanne, Michael Starzinger, Message: ptal Description: Fix for arm64 after v8:r26448 The offensive cl is https://codereview.chromium.org/874323003/ Test: ./out/arm64.debug/d8 --test --random-seed=-235865360 --turbo-deoptimization --turbo-filter=* --always-opt --debug-cod

[v8-dev] Remove obsolete method ProfileTree::AddPathFromStart and the corresponding test. (issue 907353003 by loi...@chromium.org)

2015-02-10 Thread loislo
Reviewers: Sven Panne, alph, Michael Starzinger, Message: PTAL Description: Remove obsolete method ProfileTree::AddPathFromStart and the corresponding test. BUG= LOG=n Please review this at https://codereview.chromium.org/907353003/ Base URL: https://chromium.googlesource.com/v8/v8.git@mas

[v8-dev] Re: Propagate Deopt reason to cpu-profiler (issue 910773002 by loi...@chromium.org)

2015-02-10 Thread loislo
https://codereview.chromium.org/910773002/diff/180001/src/deoptimizer.cc File src/deoptimizer.cc (right): https://codereview.chromium.org/910773002/diff/180001/src/deoptimizer.cc#newcode597 src/deoptimizer.cc:597: fp_to_sp_delta_)); On 2015/02/10 14:00:27, jarin wrote: I believe it is important

[v8-dev] Re: Propagate Deopt reason to cpu-profiler (issue 910773002 by loi...@chromium.org)

2015-02-10 Thread loislo
https://codereview.chromium.org/910773002/diff/120001/src/arm64/deoptimizer-arm64.cc File src/arm64/deoptimizer-arm64.cc (right): https://codereview.chromium.org/910773002/diff/120001/src/arm64/deoptimizer-arm64.cc#newcode135 src/arm64/deoptimizer-arm64.cc:135: __ Mov(x3, Operand(ExternalReferen

[v8-dev] Propagate Deopt reason to cpu-profiler (issue 910773002 by loi...@chromium.org)

2015-02-09 Thread loislo
Reviewers: Michael Starzinger, Sven Panne, yurys, alph, Message: PTAL Description: Propagate DeoptInfo to cpu-profiler 1) Deoptimizer::Reason was replaced with Deoptimizer::DeoptInfo because it also has raw position. Also the old name clashes with DeoptReason enum. 2) c_entry_fp assignment cal

[v8-dev] Re: Simplify cpu-profiler test code with help of wrappers. (issue 892953004 by loi...@chromium.org)

2015-02-06 Thread loislo
https://codereview.chromium.org/892953004/diff/60001/test/cctest/test-cpu-profiler.cc File test/cctest/test-cpu-profiler.cc (right): https://codereview.chromium.org/892953004/diff/60001/test/cctest/test-cpu-profiler.cc#newcode53 test/cctest/test-cpu-profiler.cc:53: static v8::Handle Compile(cons

[v8-dev] Simplify cpu-profiler test code with help of wrappers. (issue 892953004 by loi...@chromium.org)

2015-02-06 Thread loislo
Reviewers: Sven Panne, Michael Starzinger, alph, Message: PTAL The patch is mostly cosmetic and has a few fixes in string comparison. Description: Simplify cpu-profiler test code with help of wrappers. BUG=none LOG=n Please review this at https://codereview.chromium.org/892953004/ Base URL:

  1   2   3   4   >