[SPARK-21254] [WebUI] History UI performance fixes #18783
Closed
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
…ore processing Currently all the DOM manipulations are handled in a loop after Mustache template is parsed. This causes severe performance issues especially within loops iteration over thousands of (attempt/application) records and causing all kinds of unnecessary browser work: reflow, repaint, etc. This could be easily fixed by preparing a DOM node beforehand and doing all manipulations within the loops on detached node, reattaching it to the document only after the work is done. The most costly operation in this case was setting innerHTML for `duration` column within a loop, which is extremely unperformant: https://jsperf.com/jquery-append-vs-html-list-performance/24 While duration parsing could be done before mustache-template processing without any additional DOM alteratoins.
Check whether to display pagination or not on large data sets (10-20k rows) was taking up to 50ms because it was iterating over all rows. This could be easily done by testing length of array before passing it to mustache.
Logic related to `hasMultipleAttempts` flag: - Hiding attmptId column (if `hasMultipleAttempts = false`) - Seting white background color for first 2 columns (if `hasMultipleAttempts = true`) was updating DOM after mustache template processing, which was causing 2 unnecessary iterations over full data set (first through jquery selector, than through for-loop). Refactoring it inside mustache template helps saving 80-90ms on large data sets (10k+ rows)
Refactoring incomplete requests filter behavior due to inefficency in DOM manipulations. We were traversing DOM multiple times just to hide columns that we could have avoided rendering in mustache (end date, duration). Factoring this logic in mustache template (`showCompletedColumn`) saves 70-80ms on 10k+ rows.
Detaching history table wrapper from document before parsing it with DataTables plugin and reattaching back right after plugin has processed nested DOM. This allows to avoid huge amount of browser repaints and reflows, reducing initial page load time in Chrome from 15s to 4s for 20k+ rows
|
Looks like you opened a duplicate of #18777 |
|
It would have to go in master first so close the 2.1one |
|
@srowen Done |
ajbozarth
approved these changes
Jul 31, 2017
|
Thank you @ajbozarth |
|
It looks a lot simpler. |
srowen
reviewed
Aug 1, 2017
pom.xml
Outdated
| @@ -179,7 +179,7 @@ | |||
| <jpam.version>1.1</jpam.version> | |||
| <selenium.version>2.52.0</selenium.version> | |||
| <paranamer.version>2.6</paranamer.version> | |||
| <maven-antrun.version>1.8</maven-antrun.version> | |||
| <selenium.htmlunitdriver.version>2.27</selenium.htmlunitdriver.version> <maven-antrun.version>1.8</maven-antrun.version> | |||
There was a problem hiding this comment.
(Add back a newline at the end before the next property)
Selenium's version don't necessarily match with its htmlunitdriver? just checking
There was a problem hiding this comment.
Hi,@srowen
Is the version of "selenium htmlunitdriver" too low?
There was a problem hiding this comment.
@highfei2011 @srowen Reverted all pom.xml change since they appear not to be mandatory for this PR
Indeed it seems that selenium version should have been updated as well.
…ependency" Upgrade of HtmlUnit seems to bring new dependencies and since it is not required for the purposes of PR (`HistoryServerSuite` tests pass anyway on our Jenkins), we revert HtmlUnit version change This reverts commit 96598ab.
2ooom
commented
Aug 1, 2017
pom.xml
Outdated
| @@ -179,7 +179,7 @@ | |||
| <jpam.version>1.1</jpam.version> | |||
| <selenium.version>2.52.0</selenium.version> | |||
| <paranamer.version>2.6</paranamer.version> | |||
| <maven-antrun.version>1.8</maven-antrun.version> | |||
| <selenium.htmlunitdriver.version>2.27</selenium.htmlunitdriver.version> <maven-antrun.version>1.8</maven-antrun.version> | |||
There was a problem hiding this comment.
@highfei2011 @srowen Reverted all pom.xml change since they appear not to be mandatory for this PR
Indeed it seems that selenium version should have been updated as well.
|
Could we try to run tests and build on Jenkins? |
|
Jenkins test this please |
|
Test build #80133 has finished for PR 18783 at commit
|
|
Thank you @srowen. Seems like jenkins is happy. Should we merge in this case? |
|
Merged to master |
asfgit
pushed a commit
that referenced
this pull request
Aug 30, 2017
## This is a backport of PR #18783 to the latest released branch 2.2. ## What changes were proposed in this pull request? As described in JIRA ticket, History page is taking ~1min to load for cases when amount of jobs is 10k+. Most of the time is currently being spent on DOM manipulations and all additional costs implied by this (browser repaints and reflows). PR's goal is not to change any behavior but to optimize time of History UI rendering: 1. The most costly operation is setting `innerHTML` for `duration` column within a loop, which is [extremely unperformant](https://jsperf.com/jquery-append-vs-html-list-performance/24). [Refactoring ](criteo-forks@b7e56ee) this helped to get page load time **down to 10-15s** 2. Second big gain bringing page load time **down to 4s** was [was achieved](criteo-forks@3630ca2) by detaching table's DOM before parsing it with DataTables jQuery plugin. 3. Another chunk of improvements ([1]criteo-forks@aeeeeb5), [2](criteo-forks@e25be9a), [3](criteo-forks@9169707)) was focused on removing unnecessary DOM manipulations that in total contributed ~250ms to page load time. ## How was this patch tested? Tested by existing Selenium tests in `org.apache.spark.deploy.history.HistoryServerSuite`. Changes were also tested on Criteo's spark-2.1 fork with 20k+ number of rows in the table, reducing load time to 4s. Author: Dmitry Parfenchik <d.parfenchik@criteo.com> Closes #18860 from 2ooom/history-ui-perf-fix-2.2.
MatthewRBruce
pushed a commit
to Shopify/spark
that referenced
this pull request
Jul 31, 2018
## This is a backport of PR apache#18783 to the latest released branch 2.2. ## What changes were proposed in this pull request? As described in JIRA ticket, History page is taking ~1min to load for cases when amount of jobs is 10k+. Most of the time is currently being spent on DOM manipulations and all additional costs implied by this (browser repaints and reflows). PR's goal is not to change any behavior but to optimize time of History UI rendering: 1. The most costly operation is setting `innerHTML` for `duration` column within a loop, which is [extremely unperformant](https://jsperf.com/jquery-append-vs-html-list-performance/24). [Refactoring ](criteo-forks@b7e56ee) this helped to get page load time **down to 10-15s** 2. Second big gain bringing page load time **down to 4s** was [was achieved](criteo-forks@3630ca2) by detaching table's DOM before parsing it with DataTables jQuery plugin. 3. Another chunk of improvements ([1]criteo-forks@aeeeeb5), [2](criteo-forks@e25be9a), [3](criteo-forks@9169707)) was focused on removing unnecessary DOM manipulations that in total contributed ~250ms to page load time. ## How was this patch tested? Tested by existing Selenium tests in `org.apache.spark.deploy.history.HistoryServerSuite`. Changes were also tested on Criteo's spark-2.1 fork with 20k+ number of rows in the table, reducing load time to 4s. Author: Dmitry Parfenchik <d.parfenchik@criteo.com> Closes apache#18860 from 2ooom/history-ui-perf-fix-2.2.
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.
What changes were proposed in this pull request?
As described in JIRA ticket, History page is taking ~1min to load for cases when amount of jobs is 10k+.
Most of the time is currently being spent on DOM manipulations and all additional costs implied by this (browser repaints and reflows).
PR's goal is not to change any behavior but to optimize time of History UI rendering:
The most costly operation is setting
innerHTMLfordurationcolumn within a loop, which is extremely unperformant. Refactoring this helped to get page load time down to 10-15sSecond big gain bringing page load time down to 4s was was achieved by detaching table's DOM before parsing it with DataTables jQuery plugin.
Another chunk of improvements (1, 2, 3) was focused on removing unnecessary DOM manipulations that in total contributed ~250ms to page load time.
How was this patch tested?
Tested by existing Selenium tests in
org.apache.spark.deploy.history.HistoryServerSuite.Changes were also tested on Criteo's spark-2.1 fork with 20k+ number of rows in the table, reducing load time to 4s.