Use TimeGraphContainer forceCanvasRenderer option#1206
Use TimeGraphContainer forceCanvasRenderer option#1206marcdumais-work merged 1 commit intomasterfrom
Conversation
marcdumais-work
force-pushed
the
useNoWebGlContext
branch
3 times, most recently
from
ec9a853 to
35629f3
Compare
| lineColor: this.props.style.lineColor, | ||
| classNames: 'horizontal-canvas' | ||
| classNames: 'horizontal-canvas', | ||
| noWebGl: true |
There was a problem hiding this comment.
Shouldn't this container use webGL as well? This one is for marker rows (e.g. for lost events) or periodic markers? Of course this will decrease the number of available webGL contexts (up-to half less when e.g. periodic markers are shown) WDYT?
There was a problem hiding this comment.
yes, in the end it's a trade-off. I should mention, I briefly used the devtools profiling, comparing using only WebGL for all time-graph containers vs software rendering for all, and the results for the later were almost as good as the former, except if the laptop's CPU was very busy (one time i happened to be running the timeline-chart unit tests and noticed the difference).
WDYT?
There was a problem hiding this comment.
The latest version uses WebGL for the case above. Compared to master, we will now be able to have 7 or 8 time-graph charts opened at one time, spread over any number of experiments. On master, each experiment with views opened would consume 2 extra WebGL contexts, even if it has no timeline-chart based views opened.
If we used software rendering for the case above, like we had initially in the PR, it would double that number to about 15-16 timeline chart views.
marcdumais-work
force-pushed
the
useNoWebGlContext
branch
from
35629f3 to
5b4b969
Compare
marcdumais-work
changed the title
There's a relatively low limit of WebGL contexts available for a Chrome-based browser tab (16). Each time a "TimeGraphContainer" is created, one such context is consumed. We use these contexts for a few things, not just time-line charts (where it makes most sense). This commit takes advantage of the new option and declines that a WebGL context be used for containers we create, except for proper chart views. This means that we will now be able to create about 16 timeline-chart views per session, spread over however many experiments. Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work
force-pushed
the
useNoWebGlContext
branch
from
5b4b969 to
b621692
Compare
|
I wanted to mention that this PR is a first attempt to mitigate the issues caused by limited number of
|
|
Note: the license check passed locally - the workflow seems to fail because of too many requests towards |
bhufmann
left a comment
bhufmann
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks!
|
Thanks for the review! |
2 participants
What it does
There's a relatively low limit of WebGL contexts available for a Chrome-based browser tab (16). Each time a "TimeGraphContainer" is created, one such context is consumed. We use these contexts for a few things, not just time-line charts (where it makes most sense).
This commit takes advantage of the new option and declines that a WebGL context be used for containers we create, except for proper timeline chart views and marker containers. This means that we will now be able to create about 8 timeline-chart views per session (each uses 2 WebGL contexts), spread over however many experiments.
Note: Depends on the following timeline-chart PR, where theforceCanvasRendereroption is added in theTimeGraphContainerOptionsinterface:eclipse-cdt-cloud/timeline-chart#318
update: merged and released
How to test
The easiest would be to wait until the PR above is merged and released. in the meantime,
yarn linkcould be used.Follow-ups
Review checklist