mraleph / irhydra

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

Some thoughts on the UI #16

Open mraleph opened 10 years ago

mraleph commented 10 years ago

Originally filed at https://code.google.com/p/irhydra/issues/detail?id=37

DISCLAIMER: This issue is meant mostly as a discussion piece rather than critique or a specific issue with the current UI. These are my thoughts on the IRHydra 2 UI, after having spent some time watching the introductory video and playing around with it for a bit. This means I have virtually no experience in using this app, and these are just sort of immediate knee-jerk reactions to first usage. Please take this with a sufficient amount of salt, since more experienced users may very well disagree.

It should also be noted that all of these comments are from testing the app in Chrome. I have not tried other browsers.


First things first: this application looks really nice, kudos! I particularly like the efficient use popups to provide explanations of symbols and deoptimizations inline.

Popups

One thing that caught my eye however is that the deopt button in the header acts differently from the deopt buttons inline. The one in the header reacts on hover and disappears on mouse out; the one inline reacts on click and acts as a toggle, removing the popup only when the button is clicked again. Additionally, the inline deopt popup doesn't include the same explanatory text as the header one. This inconsistency was a bit confusing to me. The popups on symbols also act only on hover.

The popups seems to always display in the top left corner of the application. This is also a bit confusing, and I wonder if it wouldn't make more sense to position the popup inline with the symbol/deopt to which it belongs. Another option might be to use the space to the right of the disassembly view for explanatory text, along with some pointer to the specific element the text refers to.

Graph

The graph provides a really nifty view of the execution flow, I think. Again, the use of popups to provide meaningful context to the otherwise non-descriptive names (B0, B1 etc.) is very good and means there's little need for any more descriptive identifiers to the nodes. The same general notions above regarding popup placement apply here as well though. One thing I really miss in this view is a legend to describe what the different color boxes, border styles and arrows mean.

I really like this view, because it provides so much clarity while still being fairly concise; more information is provided as the user drills down using various actions (hover over a block, click on it to change to IR view.) At least to me as a novice, this view provided a clear overview allowing for deeper inspection by drilling down into specific blocks.

Source

Again, great use of inline popups to provide contextual views of inlined functions and deoptimizations. The same things that I've already mentioned regarding popup placement and such applies here.

One thing that took me a while to notice was the breadcrumb list. It might be worth considering whether this can be sufficiently separated from the other elements in the header; perhaps a different background color? The trailing much-greater-than sign is a bit confusing; it made me think the last item in the breadcrumb list was actually the parent to what I was looking at. In any event, I like that the breadcrumbs exist. It makes it a lot easier to see context at a glance, but I think it can be more pronounced.

Other than that, the only thing that really caught my eye was how different this view is to the IR view. There are no line numbers, the type is set in a larger size. I presume the larger size is to make the icons look less out of place. From a style point of view, I'd consider trying to make the typography more consistent, but I doubt this is more than nitpicking.

Method list

I mentioned on twitter that a large list of items that require scrolling is probably not the right view, and I maintain that standpoint. I didn't find the large list of methods very useful, because the only ones I actually really cared for where the ones where deoptimizations occur. Even then, it's possibly not very useful, because if I look at demo 1 the first loop entry is red but the deoptimization occurs in len. This hierarchy isn't apparent in a flat list like the method list, and from the perspective of finding performance issues I didn't find it very useful at all. It also includes noise such as valueOf, IsPrimitive and toString. The noise is even more apparent with lots of anonymous methods as evident in the introductory video. I've almost certainly not understood the point of this list, and to me it provided little if any real navigational value. I'm not sure what the order is, whether the order is significant in some way, or how things in the list relate to one another.

I don't know what, if anything, makes more sense than a flat list of methods. The other views worked well for drilling down and navigating, and I seemed to mostly use the list on the left to navigate to the "entry point" methods, as it were. I generally seemed to ignore instance methods, because they provide no context.

Perhaps by changing the purpose of the list, it can provide more utility? I'm thinking, instead of a flat list of methods, it could be a list of potential bottlenecks in order of potential performance issues (top is worst, bottom is better.) Even better is if this list points to parts of the code that are responsible for trailing issues – in the demo 1 example it's v2.name, but the app only really displays the side effects of this, not highlighting that this is in fact the issue. I'm guessing this requires a lot more sophisticated analysis, but still.

The list could of course still grow to include way too many items, but wouldn't necessarily depend so directly on the size of the code base; i.e. a large project could be very well optimized so has few items, whereas a small one is terrible and so has many items.


Anyway, just some thoughts from a total novice who's probably misunderstood half of the application. Perhaps some of it is useful to you, otherwise please feel free to call me an idiot and ignore all of it! :o)