mraleph / irhydra

Tool for displaying IR used by V8 and Dart VM optimizing compilers
Apache License 2.0
434 stars 32 forks source link

UI doesn't look like at all as in the talks/demos #30

Closed ashnur closed 10 years ago

ashnur commented 10 years ago

I am trying to check some nodejs code, I run ~/node11/node-v0.11.14-linux-x64/bin/node --trace-hydrogen --trace-phase=Z --trace-deopt --code-comments --hydrogen-track-positions --redirect-code-traces --redirect-code-traces-to=code.asm --print-opt-code test/opt.js then load the 2 generated files in IRHydra2, but only 3 methods show up, and there isn't much too look at, even though the code uses much more than just those 3 methods.

mraleph commented 10 years ago

True, the UI indeed has changed a bit, compared to some older talks.

I just built a fresh node v0.11.14 and tried it out - seem to work just fine for me.

screen shot 2014-10-17 at 12 35 18 am

Make sure that you are loading the right hydrogen CFG file. With command line like that it will have mangled name hydrogen-<pid>-<id>.cfg. If you remove --redirect-code-traces-to=code.asm from the command line then code file will be mangled in the same way (code-<pid>-<id>.asm) so it will be easier to select the right files.

Now, if only 3 methods show up it can mean then only those 3 methods are optimized. Keep in mind that unoptimized stuff will not show up.

I need more info to reproduce the issue (if there is one).

I don't use IRHydra with node myself, but @trevnorris does. Trevor, do you have an idea whether IRHydra2 works with v0.11.14?

ashnur commented 10 years ago

Hi, thank you for your explanations, I have progress! :) ( I was a bit afraid that this project is abandoned, and no one uses it anymore, or something terrible like that :-S )

After reading what you wrote I took the assumption that maybe the require calls are not a good thing and I created a single file to be used. With that I finally got more methods listed than just the previous three, so this is getting closer what I was expecting.

However I still don't see those deopt tags anywhere, but grepping for it in the files gives lots of results.

I created a repository with the file I am using and my hydrogen* and code.asm files, if an expert might have time to give it a glance and tell me what I am doing wrong. https://github.com/ashnur/arb-optimizations

mraleph commented 10 years ago

There are no deoptimizations in those files, so none are displayed :) Just search for DEOPT in the code.asm to confirm.

I fixed one bug though revealed by your files 6772db8c456013076e6ef95122995668743dd1ac (it was impossible to view function add due to this).

I am closing this issue, but feel free to ask more questions here!

ashnur commented 10 years ago

That's a very obvious reason, which I didn't even consider. (I just grepped for -i 'deopt')

On one hand this is great, but on the other it's terrible because now I am out of ideas on how to speed the lib up even more. :( Some of these crypto bigint libs are beating me (using their own benchmarks, but still, I was hoping to get out more from TypedArrays.

Anyhow, thank you for the help and for the great tool!

trevnorris commented 10 years ago

IRHydra does work on v0.11.14. It broke momentarily when we upgraded to V8 v3.28 but it was fixed by applying @mraleph fix (https://github.com/joyent/node/commit/270e998c9).

ashnur commented 9 years ago

I had a break from the project where I used IRHydra, and now when I got back to it, I re-read your response, and I realized something which I missed before, even though it's quite important.

Now, if only 3 methods show up it can mean then only those 3 methods are optimized. Keep in mind that unoptimized stuff will not show up

Does this mean, that methods which are not optimized are slow? If that's the case, is there any way to make them optimized? Writing them differently or something like that?

mraleph commented 9 years ago

@ashnur That depends on why they are not optimized. It can be that they contain something V8 can't optimize (e.g. try { } catch (e) { } or with (v) { } etc) or they are just not hot - which means not called enough times. If they are not hot it does not matter whether they are optimized or not.

If V8 doesn't optimize a function because it contains unsupported syntactic construct - you usually can't easily see it. Sometimes however V8 tries to optimize a method and then discovers it can't - in this case it will print the reason in --trace-opt output. You will have to look for it manually. IRHydra can't help you here unfortunately.

ashnur commented 9 years ago

Right, so I wasn't running it enough times. I made a loop with many thousands of iterations and suddenly everything is there! Again, running with my default node (v0.10.35) is giving me very confusing outputs - it puts lot of of DEOPTs into the command line which then I can't find in IRHydra² - but ignore that, I just downloaded (again) a dev version of node (v0.11.15) and that is working much nicer, even with --print-opt-code .

After this I loaded the code.asm and hydrogen-24488-1.cfg files and started browsing around. I have two questions:

And maybe just one more question.. some of the graphs are quite large and complicated - as my functions are sometimes large and v8 does inline them into each other. I have a hunch that this is also not good, that the graphs shouldn't "fork" too many times, is this true or I am just very bad at guessing about things I don't understand?

mraleph commented 9 years ago

Not having a block in IR view that is there in the graph view sounds like a bug. Check the console - maybe there is an exception during IR pane rendering. I'll appreciate a repro if you have one.

btw there is a legend pane now for the graph view, click on the question mark that is next to pane selector - there you can see what purple borders mean among some other things.

changes[*] means V8 doesn't know what instruction is changing (e.g. it can be a call). V8 optimizer is mostly intraprocedural so it does not know what happens in other functions. It's not "allseeing" - you might now what all of your functions do and what side effects they have, but when V8 optimizes them it does it one at a time in isolation. That's why there are all these changes[*] (dashed red borders on blocks).

Whether it's a good or a bad thing depends on the code. If there is a call in a loop that is otherwise tight and the callee is a small function - then this is bad. Otherwise - hard to say :)

That's why V8 agressively inlines things. When you take the callee and inline it into the caller then call goes away and optimizer sees what's happening inside the callee - allowing it bigger insight into the side-effects it has.

Whether inlining is helping or not again depends on the concrete code. Usually inlining helps.

trevnorris commented 9 years ago

@ashnur Node v0.10.x doesn't work with IRHydra2.

ashnur commented 9 years ago

@mraleph Sorry that I wasn't clear enough, obviously I read the legend and the mouseover/tooltip information on what those borders/colors mean. What I was not sure about is this:

Whether it's a good or a bad thing depends on the code. If there is a call in a loop that is otherwise tight and the callee is a small function - then this is bad. Otherwise - hard to say :)

Sometimes it's strange things, like I from inside Math.floor , Number, String.slice(still number related things), other times it's not even a function call just setting a value in a TypedArray. I am guessing maybe it's changing between int and float representations?

Check the console - maybe there is an exception during IR pane rendering. I'll appreciate a repro if you have one. Yes, there are some errors, is it enough if I just upload the code.asm and hydrogen.cfg (and maybe source) files somewhere?

@trevnorris thanks, I wasn't sure about that since this page http://mrale.ph/irhydra/2/ mentions node v0.10 in two links. However there is no mention of node v0.11 working so nicely together with IRHydra² that even the --print-opt-code options is working.

mraleph commented 9 years ago

Sometimes it's strange things, like I from inside Math.floor , Number, String.slice(still number related things), other times it's not even a function call just setting a value in a TypedArray.

I am not sure I can complete understand what happens to Math.floor etc, but if you have a KeyedStoreGeneric emitted for a store arr[i] = smth where arr is a typed array then this is either polymorphism, lack of type feedback or you might be storing with strange indices (out of bounds, strings instead of numeric indices etc).

this page http://mrale.ph/irhydra/2/ mentions node v0.10

Yeah, true. So it used to work at least partially (if you click on those links - you will see that SOURCE feature doesn't work). I think those traces were obtained on a clean build of node, without any modifications.

ashnur commented 9 years ago

if you have a KeyedStoreGeneric emitted for a store arr[i] = smth where arr is a typed array then this is either polymorphism, lack of type feedback or you might be storing with strange indices (out of bounds, strings instead of numeric indices etc).

It's StoreKeyedGeneric and you are (of course) right, I was trying to be too clever I guess and using the store into the TypedArray for modulo operation. -- edit: ok, now that I read it again, I am definitely not using out of bounds indices or string indices, just storing a value which is larger than the entry size

I also uploaded all the files here https://github.com/ashnur/arb-optimizations in case you want to check those console errors, they appear for me on the method sub (Uncaught RangeError: Value not in range (1))

mraleph commented 9 years ago

just storing a value which is larger than the entry size

Hmm

also uploaded all the files here

Thanks for the repro. It's Issue #38 which I am not sure how to fix in a backwards compatible way. I think I will have to just say "I only support most recent V8 versions".

ashnur commented 9 years ago

Well, I will have to learn how to compile V8 with the right flags then. I gave it a shot today, but it was far more complicated than I hoped for so I just went back to node11. Will try it again tomorrow.

mraleph commented 9 years ago

@ashnur I think it should just work now. At least it works now for me. Source position markers fall into correct places now.

screen shot 2015-01-22 at 11 40 03 pm

I need to check how three most recent versions of V8 behave to verify that this fixes it. Will do it tomorrow.

ashnur commented 9 years ago

It works for me too, thank you very much. I am also starting to understand what it says, I already see a bunch if things I can debug.

mraleph commented 9 years ago

I checked that IRHydra would work with V8 3.29 - 4.1 - last three versions. Which is I think what I should target. I would change wording on the index page to highlight this.