Improve FF performance by only having one place to set the y-axis width #6492
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'. |
|
jenkins, test it |
|
@stormpython can you review this and make sure that the spacing of the y-axis remains correct |
|
@rashidkpc ok. |
|
@trevan in Chrome, I am seeing staggered y axes.
|
|
Same thing occurs in Firefox when splitting charts.
|
|
Yeah, I just noticed that yesterday. I'm looking into what is causing it. |
|
@trevan did you ever figure out what was going on with this? |
|
Doesn't look like there's any interest in this anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have a dashboard that has 11 line graphs. In FF, the browser hangs during the graph rendering and would usually ask me if I want to kill the process. Running a performance report during the page load showed that a lot of the time was taken up doing width calculations (either setting or getting).
It led me to the setWidth function in vislib/lib/layout/splits/column_chart/y_axis_split.js and after removing it, performance increased quite a bit. After noticing that without that function, the width of the y-axis was still being set, I tracked down the code in vislib/lib/y_axis.js that appears to do the same work. The only difference is the padding. In the y_axis_split.js, it adds a padding of 5 twice so I added a padding of 10 to y_axis.js.
Here's the performance graph before the change. The period between 9600ms and 16000ms is the rendering of the graph. It shows an fps of roughly 0 during that time period.
Here's the performance graph after the change. The period between 9600ms and 12000ms is the rendering of the graph. This change shaves off 4 seconds in rendering time.
I don't think this change breaks anything but I've only looked at line charts and bar charts. I've also checked it in Chrome and it doesn't seem to affect it that much.