Ideas to improve perf on modern browsers #14

deviousasti · 2018-07-07T19:29:20Z

I noticed that a lot of measurement and subsequent layout recalculation is taking place in the animation frame. A lot of measured values can be cached, and perf can be improved by batching reads and writes.

For eg., the line 5563:
label.style.width = width + 'px'; // set width to prevent overflow

takes a whole 6ms (on a Ryzen 7). I'm combining this with a canvas, and its framerates end up being killed every time the timeline is moved.

Just started an issue to list ideas here.

The text was updated successfully, but these errors were encountered:

yotamberk · 2018-07-08T16:26:39Z

Awesome! Thanks for starting this thread

deviousasti · 2018-07-08T18:17:38Z

Suggestions:

Combine style.left = x and style.top = y into style.transform = translate(x, y) (this change singlehandedly makes rendering a lot smoother)
Convert element.innerHTML to element.innerText
Use native methods whenever possible:

  vis.util.addClassName = function (elem, classNames) {
        elem.classList.add(...classNames.split(' '));
    }

    vis.util.removeClassName = function (elem, classNames) {
        elem.classList.remove(...classNames.split(' '));
    }

    vis.util.forEach = function forEach(object, callback) {
        if (Array.isArray(object)) {
            // array
            object.forEach(callback);
        } else {
            // object
            Object.getOwnPropertyNames(object).forEach(i => callback(object[i], i, object));
        }
    }

I'll edit this list to add more. 😃

yotamberk · 2018-07-08T18:34:40Z

Hi @deviousasti ,
Let me comment about the different points:

transform is indeed MUCH more efficient than left and top since it uses repaint without layout/redraw. BUT since all the code is based left and top, this will be extremely difficult to change.
It might be possible to be applied in chunks starting with the most crucial components, such as the different Item types (RangeItem, BackgroundItem etc.) This was an old idea of mine a while back. I might give it a shot the next few weeks.
converting this will have consequences since most of the uses of element.innerHTML is to allow the user to inject his own html elements. I will review these points carefully to see where this can be safely swapped to element.innerText
Can you give more specific locations where you would use these?

Awesome points! Thanks! Keep up with the ideas!

deviousasti · 2018-07-08T19:56:21Z

Hi @yotamberk,

About #1, I've simply replaced all instances of setting the left property with transform and it works great.
About #3, setting the major, minor labels - css classes and the formats of the labels are user-definable anyway, and can safely be changed to innerText.

I modified the dist file with the above changes and it works fine.
(https://www.dropbox.com/s/dppedw1x47fa48n/timeline.js?dl=0)

Avoid creating new Date() - getting the system date-time is very expensive and the date constructor has terrible perf. Even Date.now is 50% faster. (https://jsperf.com/date-now-vs-new-date)

Sections like these are abuse:

              function currentWeek(date) {
                return date.isSame(new Date(), 'week') ? ' timeline-current-week' : '';
              }

              /**
               *
               * @param {Date} date
               * @returns {String}
               */
              function currentMonth(date) {
                return date.isSame(new Date(), 'month') ? ' timeline-current-month' : '';
              }

              /**
               *
               * @param {Date} date
               * @returns {String}
               */
              function currentYear(date) {
                return date.isSame(new Date(), 'year') ? ' timeline-current-year' : '';
              }

deviousasti · 2018-07-08T20:15:51Z

I think most importantly, to hit 60 fps - and it should be very feasible:

Cache offsetWidth, offsetHeight, clientWidth and clientHeight.

Refactor functions like:

 function getAbsoluteLeft(elem) {
   return elem.getBoundingClientRect().left;
 }
 function getAbsoluteTop(elem) {
   return elem.getBoundingClientRect().top;
 }

Lines like:
this.popup.setPosition(event.clientX - util.getAbsoluteLeft(container) + container.offsetLeft, event.clientY - util.getAbsoluteTop(container) + container.offsetTop); are terrible offenders, calculating bounds twice.

Sam-Sutherland · 2018-07-09T11:37:01Z

This is great! The proposed changes have pretty good performance increases. There are a few issues with label stacking at the top then at the bottom if you set orientation: {axis: "top"} (Shown Below) but other than that it looks good.

I've had a brief play around with getting the RangeItem to use transform as well but weird stuff was happening to the backgrounds.

Looking forward to seeing this progress!

deviousasti · 2018-07-16T05:35:32Z

Guys, need any help implementing this?

yotamberk · 2018-07-16T07:42:21Z

PRs are always welcomed! I don;t have the time to implement this right now.

jarnovanrhijn · 2019-05-13T14:51:28Z

Guys, need any help implementing this?

@deviousasti, have you ever gotten to writing a pull request? otherwise i'll look into it myself :) and try to make a pull request.

deviousasti · 2019-05-14T04:13:45Z

@jarnovanrhijn I wasn't able to build it before - I tested out those on the built version.
I'll try again.

deviousasti · 2019-05-14T05:39:44Z

@jarnovanrhijn I just added a few changes to
https://github.com/deviousasti/timeline-plus/commits/master
Help me out?

@yotamberk
In BoxItem there's repositionX and repositionY - these should ideally be combined into one translate move - this is a breaking change. Thoughts?

jarnovanrhijn · 2019-05-14T06:34:29Z

@jarnovanrhijn I just added a few changes to
https://github.com/deviousasti/timeline-plus/commits/master
Help me out?

@yotamberk
In BoxItem there's repositionX and repositionY - these should ideally be combined into one translate move - this is a breaking change. Thoughts?

@yotamberk,
I'm not sure if the performance will increase much if combined as this would only update the correct coords right? isn't translateX and translateY an option to avoid a major change for now? only caveat here is that we need to know the current translateX and translateY as they are used on the same property.

deviousasti · 2019-05-14T06:54:36Z

It will be sort of hacky to have it separate. Can try.

jarnovanrhijn · 2019-05-14T10:54:56Z

It will be sort of hacky to have it separate. Can try.

Let me know if i can do anything to help!!

taavi-halkola · 2019-05-14T15:22:49Z

Most likely the easiest choice would be to check if both the top and left values are present when updating the translate value.

In repositionx check if this.top is already set. If not then only do translatex. If it is then do translate. The same goes to repositiony but other way around.

Also another big problem in redraw performance is the forced reflows from calculating bounds, offsetheights, offsetwidths or changing inline styles to early in the dom tree nodes.. anyone checked into those? One example is the cursor change when the timeline scrolling is started. I think that style change alone causes a reflow to all children. Mostly the problems are from getting offsetwidths, offsetheights, offsetlefts and offsettops for groups and items.

deviousasti · 2019-05-14T16:39:08Z

I’ve looked at some of that. Offset calculation is the one of the biggest bottlenecks. We might need to batch reads and writes to avoid layout thrashing. Not sure if we need something like fastdom. Good momentum in this thread. Sent from Mail for Windows 10 From: Taavi Halkola Sent: Tuesday, May 14, 2019 8:52 PM To: yotamberk/timeline-plus Cc: Asti; Mention Subject: Re: [yotamberk/timeline-plus] Ideas to improve perf on modernbrowsers (#14) Most likely the easiest choice would be to check if both the top and left values are present when updating the translate value. In repositionx check if this.top is already set. If not then only do translatex. If it is then do translate. The same goes to repositiony but other way around. Also another big problem in redraw performance is the forced reflows from calculating bounds, offsetheights, offsetwidths or changing inline styles to early in the dom tree nodes.. anyone checked into those? One example is the cursor change when the timeline scrolling is started. I think that style change alone causes a reflow to all children. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jarnovanrhijn · 2019-05-14T16:54:30Z

Made a pull request which fixes the multiple calculatings.

Functions like getAbsoluteTop and getAbsoluteLeft are changed to a single util function and cached when used.

Will look into the layout trashing as well!! Great to seee this alive again

deviousasti · 2019-05-14T17:30:45Z

Replaced the vis.util methods with native ones -
If anyone wants this to run on really old browsers they can relegate the compat to polyfills.

I merged @jarnovanrhijn 's changes as well. Good find.
I will try to tackle BoxItem next.

jarnovanrhijn · 2019-05-14T19:42:16Z

tried to replace new Date() with Date.now() and tried to cache some heights and widths.. pretty sure i'm not there yet so i'll finish the rest tomorrow!!

jarnovanrhijn · 2019-05-16T06:37:26Z

@deviousasti & @taavi-halkola
need some help with caching the heights and widths. i think i have the quick wins covered but i think there are plenty improvements to be made regarding layout trashing.

deviousasti · 2019-05-16T07:44:19Z

@jarnovanrhijn I rewrote the positioning code for BoxItem. Please have a look. (We now have transforms!)
Merged your code in.
We somehow need to batch all reads and writes to the dom- we should move in that direction.

jarnovanrhijn · 2019-05-16T07:53:17Z

@jarnovanrhijn I rewrote the positioning code for BoxItem. Please have a look. (We now have transforms!)
Merged your code in.
We somehow need to batch all reads and writes to the dom- we should move in that direction.

The transforms alone will improve the performance allot!! Thank you for that 😄

regarding the batch read / write i think fastdom will do the trick.. although i'm not familiar with the API it shouldn't be that hard implementing it!

deviousasti · 2019-05-16T08:16:28Z

I'll have a look at removing nodes and GC as well.

And the throttle function could use another look.

deviousasti · 2019-05-17T05:36:30Z

All those small blue blocks at the bottom are HTML parses - I think we should limit it to text for tick labels. Most effects custom can be achieved with CSS if need be.

jarnovanrhijn · 2019-05-20T09:24:27Z

I've replaced left (Core.js) with translate. this will be used when verticalScroll and maxHeight are present

deviousasti · 2019-05-20T17:37:29Z

Nice! Sent from Mail for Windows 10 From: Jarno van Rhijn Sent: Monday, May 20, 2019 2:54 PM To: yotamberk/timeline-plus Cc: Asti; Mention Subject: Re: [yotamberk/timeline-plus] Ideas to improve perf on modernbrowsers (#14) I've replaced left (Core.js) with translate. this will be used when maxHeight is present! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jarnovanrhijn · 2019-05-22T14:58:26Z

@deviousasti & @yotamberk

I've added support for RTL transforms. Could you review the changes as i'm not 100% confident of the changes.

Also, locally i've tried to use fastdom for some parts of the timeline and it indeed increases performance allot. although i think everything is too tightly coupled to do it properly.

Any ideas on this topic?

deviousasti · 2019-05-22T17:15:20Z

@jarnovanrhijn Nice. RangeItem has repositionY which needs to be combined to one single translate.

The rtl support, I'm not sure. The reason why translateX works is because it's already referenced to a left-top origin, and translate moves it relative to the absolute position of that element (which by default is 0, 0).
For rtl to be translated, you will need to take the size of the relative container into account.
I made this example to explain the differences:
https://codepen.io/anon/pen/byaMYm

I hope this makes it clearer.

jarnovanrhijn · 2019-05-23T05:48:59Z

@deviousasti, Yeah i thought so as well. did some research and found https://rtl-css.net/tutorial/quick-tip-rtling-css3-transform-functions-translate-and-translatex which basically says make the X value a negative number. did some tests and it seems to work in some places but i can find myself in your explanation!

Will revert it for now till we figure out a permanent solution!

deviousasti · 2019-05-23T05:55:35Z

The alternative is to take into account the relative ‘parent’ and calculate from there, but then there’s not much perf gains to be had if you’re doing that. That’s why I left the rtl implementation as-is. I’ll just make everything use the repositionXY function, and we’ll figure out some better implementation later. Sent from Mail for Windows 10 From: Jarno van Rhijn Sent: Thursday, May 23, 2019 11:19 AM To: yotamberk/timeline-plus Cc: Asti; Mention Subject: Re: [yotamberk/timeline-plus] Ideas to improve perf on modernbrowsers (#14) @deviousasti, Yeah i thought so as well. did some research and found https://rtl-css.net/tutorial/quick-tip-rtling-css3-transform-functions-translate-and-translatex which basically says make the X value a negative number. did some tests and it seems to work in some places but i can find myself in your explanation! Will revert it for now till we figure out a permanent solution! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jarnovanrhijn · 2019-05-23T07:09:59Z

@deviousasti, Thanks for all the work man!! really happy these improvements are being made!

jarnovanrhijn · 2019-05-24T08:19:02Z

@deviousasti @yotamberk,

I had a first attempt in including fastdom in the Timeline and even though i i'm sure i did it incorrectly (hasty) i've seen major improvement in performance! this is just a test so i don't expect it to be production ready but just seeing the performance improve made me happy :D!

i've created a branch in my fork which you can try and see for yourself. just run the stressPerformance.html in there and compare it with the master branch! Without fastdom my browser crashes (5000 items and 400 groups) whilst with fastdom it runs smooth as butter!

yotamberk · 2019-06-26T19:27:32Z

Closed and implemented with the great work of @jarnovanrhijn with #168

jarnovanrhijn · 2019-06-26T22:50:13Z

I’m sorry for the late reply but I’m on Holiday and don’t have my laptop with me!!

most of the credits go to @deviousasti though, he has made most of the improvements possible 😁👌🏻

Thanks for completing the final steps needed for this merge

MonasteryJohn · 2019-06-27T15:49:08Z

@yotamberk very excited about these changes! Any idea when an update will be available in NPM?

yotamberk · 2019-06-27T19:26:14Z

This weekend \ tomorrow

yotamberk · 2019-06-29T14:19:43Z

Version has been updated to v2.4.0

yotamberk added the perf improvment label Sep 18, 2018

yotamberk closed this as completed Jun 26, 2019

Ideas to improve perf on modern browsers #14

Ideas to improve perf on modern browsers #14

Comments

deviousasti commented Jul 7, 2018

yotamberk commented Jul 8, 2018

deviousasti commented Jul 8, 2018 • edited Loading

yotamberk commented Jul 8, 2018

deviousasti commented Jul 8, 2018 • edited Loading

deviousasti commented Jul 8, 2018

Sam-Sutherland commented Jul 9, 2018

deviousasti commented Jul 16, 2018

yotamberk commented Jul 16, 2018

jarnovanrhijn commented May 13, 2019

deviousasti commented May 14, 2019

deviousasti commented May 14, 2019

jarnovanrhijn commented May 14, 2019

deviousasti commented May 14, 2019

jarnovanrhijn commented May 14, 2019

taavi-halkola commented May 14, 2019 • edited Loading

deviousasti commented May 14, 2019 via email

jarnovanrhijn commented May 14, 2019

deviousasti commented May 14, 2019 • edited Loading

jarnovanrhijn commented May 14, 2019

jarnovanrhijn commented May 16, 2019

deviousasti commented May 16, 2019

jarnovanrhijn commented May 16, 2019

deviousasti commented May 16, 2019

deviousasti commented May 17, 2019

jarnovanrhijn commented May 20, 2019 • edited Loading

deviousasti commented May 20, 2019 via email

jarnovanrhijn commented May 22, 2019 • edited Loading

deviousasti commented May 22, 2019 • edited Loading

jarnovanrhijn commented May 23, 2019

deviousasti commented May 23, 2019 via email

jarnovanrhijn commented May 23, 2019

jarnovanrhijn commented May 24, 2019 • edited Loading

yotamberk commented Jun 26, 2019

jarnovanrhijn commented Jun 26, 2019

MonasteryJohn commented Jun 27, 2019

yotamberk commented Jun 27, 2019

yotamberk commented Jun 29, 2019

deviousasti commented Jul 8, 2018 •

edited

Loading

deviousasti commented Jul 8, 2018 •

edited

Loading

taavi-halkola commented May 14, 2019 •

edited

Loading

deviousasti commented May 14, 2019 •

edited

Loading

jarnovanrhijn commented May 20, 2019 •

edited

Loading

jarnovanrhijn commented May 22, 2019 •

edited

Loading

deviousasti commented May 22, 2019 •

edited

Loading

jarnovanrhijn commented May 24, 2019 •

edited

Loading