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

Speed up the connection DB #2254

Merged
merged 2 commits into from Feb 4, 2019
Merged

Speed up the connection DB #2254

merged 2 commits into from Feb 4, 2019

Conversation

rachel-fenichel
Copy link
Collaborator

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Significantly speeds up adding and removing connections from the connection DB. This is especially important when dragging into or out of a large stack, because it's possible that all connections in the stack will need to be moved. This also makes insertion markers faster.

Dragging a block out of the spaghetti stack is now about a second faster (!)

Proposed Changes

Change the connection DB. Instead of attempting to subclass Array, we made it contain an actual array.

Reason for Changes

This means that array operations like splice are optimized by the browser/hopefully happening in C, and we're getting massive speed-up when the connection DB is large (> 1000).

The connection DB was always supposed to be an Array, and the comments will tell you that it was one. db instanceof Array would return true, but Array.isArray(db) would return false.

More details on subclassing Array here for the curious.

Test Coverage

Tested in the playground on Chrome, with both spaghetti and normal dragging. I also updated JSunit tests, which are looking at the db internals.

Additional Information

NeilFraser and others added 2 commits February 1, 2019 15:25
Previously “Array.isArray(connectionDB)” was false, even through “connectionDB instanceof Array” was true.
Copy link
Contributor

@RoboErikG RoboErikG left a comment

Choose a reason for hiding this comment

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

Nice!

@rachel-fenichel
Copy link
Collaborator Author

(Shout-out to @BeksOmega because I found this while getting screenshots explaining how I use chrome developer tools to keep an eye on performance)

@rachel-fenichel rachel-fenichel deleted the connection-array branch February 4, 2019 19:11
@rachel-fenichel rachel-fenichel mentioned this pull request Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants