Skip to content

Refactor C++, misc. C++ fixes#1233

Merged
texodus merged 5 commits intomasterfrom
misc-fix
Oct 25, 2020
Merged

Refactor C++, misc. C++ fixes#1233
texodus merged 5 commits intomasterfrom
misc-fix

Conversation

@sc1f
Copy link
Contributor

@sc1f sc1f commented Oct 23, 2020

This PR contains a few performance-based refactors: the removal of cell delta/step delta from 0-sided contexts and the public API, use of direct vector indexing whenever possible, and the removal of minmax calculations (which caused a 30-40% decrease in memory allocated per View.

Changelog

  • Change instances of push_back to reserve() and direct vector indexing whenever possible, i.e. the vector length is always known at runtime
  • Removes minmax calculation from all contexts, which used to be still enabled for Python but disabled for Javascript
  • Removes {mode: "cell"} from on_update, as row deltas are faster by orders of magnitude, and provide for the same function—a cell delta and a row delta from the same update will create equivalent updates when applied to a new table.
  • Adds tests around restore and clear when computed columns are created
  • Splits the Javascript websocket implementation into two files - client and manager, which allows it to more closely mirror the organization of the Python Tornado handler.

@sc1f sc1f added C++ internal Internal refactoring and code quality improvement labels Oct 23, 2020
@sc1f sc1f self-assigned this Oct 23, 2020
@sc1f sc1f requested a review from texodus October 23, 2020 23:01
Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the PR!

@texodus texodus merged commit 67baf50 into master Oct 25, 2020
@texodus texodus deleted the misc-fix branch October 25, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ internal Internal refactoring and code quality improvement

Development

Successfully merging this pull request may close these issues.

2 participants