Skip to content

Conversation

@skshetry
Copy link
Collaborator

@skshetry skshetry commented Feb 15, 2023

A low hanging fruit that reduces 20% of runtime when run in vscode-dvc-demo with:

dvc plots diff $(git log --oneline --pretty=format:"%h")

Screenshot from 2023-02-15 13-11-43

@skshetry skshetry added the optimize Optimizes DVC label Feb 15, 2023
@skshetry skshetry requested a review from daavoo February 15, 2023 07:30
@skshetry skshetry added the A: plots Related to the plots label Feb 15, 2023
@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Base: 93.07% // Head: 93.06% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (c417478) compared to base (6f87c14).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9033      +/-   ##
==========================================
- Coverage   93.07%   93.06%   -0.02%     
==========================================
  Files         456      456              
  Lines       36814    36758      -56     
  Branches     5335     5324      -11     
==========================================
- Hits        34264    34207      -57     
- Misses       2025     2026       +1     
  Partials      525      525              
Impacted Files Coverage Δ
dvc/render/convert.py 87.09% <100.00%> (-0.41%) ⬇️
dvc/render/converter/vega.py 91.20% <100.00%> (-0.05%) ⬇️
dvc/repo/diff.py 95.31% <0.00%> (-3.00%) ⬇️
dvc/output.py 89.18% <0.00%> (-0.30%) ⬇️
tests/func/test_diff.py 100.00% <0.00%> (ø)
dvc/repo/index.py 89.88% <0.00%> (+0.57%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@daavoo daavoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skshetry
Copy link
Collaborator Author

This is clearly another low-hanging fruit:

iterative/dvc-render@2aa47c8/src/dvc_render/vega_templates.py#L66-L71

Goes back to iterative/dvc-render#23 / iterative/dvc-render#28 (not treating the template content as string)

Thanks for the links. Seems a bit involved though, right? Does that keep backward-compatibility?

@skshetry skshetry merged commit be824ed into treeverse:main Feb 15, 2023
@skshetry skshetry deleted the avoid-deepcopying branch February 15, 2023 10:40
@daavoo
Copy link
Contributor

daavoo commented Feb 16, 2023

This is clearly another low-hanging fruit:
iterative/dvc-render@2aa47c8/src/dvc_render/vega_templates.py#L66-L71
Goes back to iterative/dvc-render#23 / iterative/dvc-render#28 (not treating the template content as string)

Thanks for the links. Seems a bit involved though, right?

I can give it a quick try as I have recently looked at it.

Does that keep backward-compatibility?

It should as there are internal operations, I/O should remain the same

@skshetry
Copy link
Collaborator Author

skshetry commented Feb 16, 2023

Thank you. FYI I ran the above benchmark with dvc pull --all-commits, so the collect() is not very pronounced.

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

Labels

A: plots Related to the plots optimize Optimizes DVC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants