Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize rendering of the study view KM charts #9988

Closed
alisman opened this issue Jan 30, 2023 · 2 comments · Fixed by cBioPortal/cbioportal-frontend#4520
Closed

Optimize rendering of the study view KM charts #9988

alisman opened this issue Jan 30, 2023 · 2 comments · Fixed by cBioPortal/cbioportal-frontend#4520
Assignees

Comments

@alisman
Copy link
Contributor

alisman commented Jan 30, 2023

https://cbioportal.mskcc.org/study/summary?id=mskimpact

The two KM charts wildly tax the resources of the browser for large samples. Scripting time, measured in profiler, is cut by 60% when we remove them. And peak memory footprint is reduced from 600MB to 300MB.

Rendering points and allowing interactivity is probably not important for this view. Perhaps we can sample and remove tooltips?

@alisman
Copy link
Contributor Author

alisman commented Feb 8, 2023

@dippindots product team agreed to getting rid of the censoring crosses and then doing binning. The way binning should work is that we put events into bins by some configurable number of days. We can apply this optimization only when there are more than say, 10k samples.

@dippindots dippindots self-assigned this Feb 9, 2023
@dippindots
Copy link
Member

dippindots commented Feb 9, 2023

@alisman @inodb @jjgao
Comments from standup meeting:

  • Remove censoring crosses for sure
  • flooring instead of binning, e.g. 100.3 months and 100.9 months fall into the same data point generated for 100 - 101 months
  • If we do flooring, modify the current tooltip to include new information: # patients censored # patients have event

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants