Skip to content

Performance and Death

David Vorick requested to merge performance into master

So...

This PR is all over the place. It's basically a disaster, but it does provide some strong performance gains. In some places, more than 2x. There are some very obvious places to improve performance further, and I fully plan on doing so in the near future.

There's also a huge downside though. Because what was required was essentially an overhaul/rewrite, I only got partway through with it. There's lots of duplicated code, and the test coverage isn't as reliable as it used to be because some of the tests follow different codepaths than the production code. All of the integration tests are still passing though, and those are pretty comprehensive. I'm fairly confident that nothing crazy-important broke.

The error handling is very inconsistent, the sanity checking isn't as strong as it used to be, and the overall codebase structure has deteriorated substantially.

I also believe that the net line count will be negative after all performance stuff is finished. It's only grossly positive right now because there's so much code duplication between the optimized vs. non-optimzed codepaths. There are also a lot of places where the disk is accessed repeatedly without necessity. For example, getting height requires going to disk right now.

This git history is also very terrible. Commits are not atomic, are littered with profiling code, and would be very temperamental to explore or revise.

Basically this is the worst pull request ever. I can continue to clean up the code if you like, but it's going to be a lot of time and effort.

These changes are here to stay. After everything is reorganized, I believe that the consensus package will be a lot stronger than it was previously. The sanity checking will be more targeted, the error checking will be more consistent, and the code structure will return to approx. its previous integrity. Basically this PR represents a massive work-in-progress and I feel confident about where it's going.

Merge request reports