Bug 219187

<rdar://problem/68871264>

Created attachment 414635 [details]
Patch

Comment on attachment 414635 [details]
Patch

When I use this patch with production data, the UI is unacceptably slow....in order to use something like this, we're going to need to improve it's performance.

Comment on attachment 414635 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414635&action=review

Overall, I think we need design a more efficient way to do this, we may need to preprocess the results, for example, group the results into different status group, then we trace where we are in that array of groups, it will be very easy to get the next group, next group will have the different status, if they have the same status as the current one, they will be merged as the same one.  What's more, we should ignore the onScroll event if a button won't be showed to the user, we can update it later when the button come in the user's sight.

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:32
> +import {DOM, EventStream, REF, FP, Signal} from '/library/js/Ref.js';

Let's don't use Signal, this is a private interface, please use EventStream instead

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:749
> +            while (results[currentResultIndex]) {

This will have a performance issue if the results is too big. and we have too many timeline to show. it will be better and faster if we preprocess it to different groups based on status. which will be faster to jump a lot of nodes

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:493
> +            setTimeout(

This may cause performance issue, option.onScrollSettle seems running a very huge load，we should limit how many those events fires

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:497
> +                        option.onScrollSettle('left', scrollLeft / scaleWidth, jumpLeftRef.element);

Directly change the element out of the setState may have a rendering issue,  it's better to provide a component of button and pass it here, then in your function, you can access that ref and use setState

Comment on attachment 414635 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414635&action=review

>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:749
>> +            while (results[currentResultIndex]) {
> 
> This will have a performance issue if the results is too big. and we have too many timeline to show. it will be better and faster if we preprocess it to different groups based on status. which will be faster to jump a lot of nodes

I don't know that pre-processing actually helps us that much.

The problem with preprocessing is we then have to run our computation of all of our data, instead of being able to pick which parts of the data we can run it on. I was attempting to limit the computation to only the bits we care about. I think we can actually do better here (ie, only compute jump indices for buttons which are currently in the view). What might be a good idea is to cache the uuid of the divisions once we find them the first time, but that only helps us so much because we still have to map uuids to scroll indices, which is not something we can cache because it's possible for us to receive commits after the first render.

>> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/library/js/components/TimelineComponents.js:493
>> +            setTimeout(
> 
> This may cause performance issue, option.onScrollSettle seems running a very huge load，we should limit how many those events fires

The idea with this function is that is could run a large load, but we keep track of how many scroll events are firing and only run the computationally expensive bits once we have had no scroll events for .1 seconds.