RFR: 6677: search to highlight cells in the flame view

[Alex Macdonald]

On Mon, 3 Feb 2020 22:04:24 GMT, Miroslav Wengner <mwengner at openjdk.org> wrote:

>> With regards to the width comment on the closed https://github.com/openjdk/jmc/pull/44#issuecomment-580854588,
>> 
>>> I can also totally remove the width scale but I've observed some not wanted behaviour on the MacOS, have you tested it on Mac ?
>> 
>> What was the unwanted behaviour you were seeing?
>> 
>> Here's my initial feedback on the PR:
>> 
>> 1. As for the sizing fix, I don't think setting the chart to 95% of the window size [[0]](https://github.com/openjdk/jmc/pull/45/files#diff-30491bb3d763f9e956043f3402596f31R88) is the complete fix; the problem is that the svg doesn't change it's size when the chart is changed. So if the chart increases in size it is cutoff by the svg, and if the chart decreases in size then it leaves excess white space. 
>> 
>> GIF1: Increasing the chart size, but the SVG doesn't change
>> ![6677-increasing-cutoff](https://user-images.githubusercontent.com/10425301/73668357-c1831c80-4673-11ea-93fd-9b6882fd81d6.gif)
>> 
>> GIF2: Decreasing the chart size, but the SVG doesn't change
>> ![6677-decreasing-excess](https://user-images.githubusercontent.com/10425301/73668366-c47e0d00-4673-11ea-87cc-ae3f61f41720.gif)
>> 
>> 2. As for the search functionality itself, I think it looks really nice. My feedback for the moment is that it is using `startsWith()` for checking the strings, so it's only possible to search by class .. it would be neat to be able to search within the string as well. e.g., search for `.run()` and have it highlight all instances of `.run()` being called.
>> 
>> [0] https://github.com/openjdk/jmc/pull/45/files#diff-30491bb3d763f9e956043f3402596f31R88
>> [1] https://github.com/openjdk/jmc/pull/45/files#diff-c309933c5cd238df4805885ae811bae1R128
> 
> @aptmac  I like your comments! 
> add1.  agree,  I know about the issue with the window. I've been searching about how to fix it properly and it looks they have opened a bug ticket in flameview. They don't properly propagate resize to the base svg. Maybe we should wait until they have it fixed, what do you think?
> ps: when you click on a new  thread to display (example ), all is properly resized according to the window size. (my observed behaviour)
> 
> add2. I've been thinking about it ! So I'll add it too let's see how performance.

> @aptmac I like your comments!
> add1. agree, I know about the issue with the window. I've been searching about how to fix it properly and it looks they have opened a bug ticket in flameview. They don't properly propagate resize to the base svg. Maybe we should wait until they have it fixed, what do you think?

I'm the one who opened that issue on the `d3-flame-graph` repo, and I've posted on one of their issues last week so I'm not sure how long it takes to get a reply (or a fix). I think if going the route of waiting for an upstream fix then we should also update to the latest version .. we're using one 1.5 years out of date at the moment. However, this would also complicate the PR for getting this to work in Windows until Chromium support is added to SWT.

In my opinion this is something that stylesheets can address and doesn't require us to rely on external parties.

> ps: when you click on a new thread to display (example ), all is properly resized according to the window size. (my observed behaviour)

Interesting, I'm only seeing a proper resize when the chart is redrawn.

![6677-resize-on-redraw](https://user-images.githubusercontent.com/10425301/73748384-509d3c80-4727-11ea-9141-8253a8f2cb57.gif)

-------------

PR: https://git.openjdk.java.net/jmc/pull/45