Skip to content

Conversation

@MazyNoc
Copy link

@MazyNoc MazyNoc commented Apr 22, 2015

Added a more optimised version of raw sql to the benchmark

@daj
Copy link
Owner

daj commented Apr 24, 2015

This looks really good, the write times for raw SQL are hugely improved (I saw a 2X to 3X improvement from a quick test run). A few suggestions:

  • Subclass the existing raw SQL test classes with your optimized SQL classes, SQLite[Optimized]Executor extends SQLiteRawExecutor and OptimizedMessage extends Message. This will remove all the duplicated code and help to highlight the differences in implementation. You can also refactor writeWholeData() so that the first half of the code is commonized between the two.
  • Add some comments explaining why SQLite[Optimized]Executor is faster. At the moment it takes quite a lot of code reading to understand it, and it would be great to make the high level explanation more clear.

I like the idea of keeping the raw SQL and Optimized SQL benchmarks separate, to show how much faster you can make the writes if you do it properly. :-)

If you aren't able to do the refactors then I might be able to find some time to help do that, but I think you would be best placed to add some comments. Thanks!

@MazyNoc
Copy link
Author

MazyNoc commented May 11, 2015

Thanks for the feedback, Ill probably have time during next week, so Ill
take a look at it then.
I also want to add DBFlow to the mix since that's a really fast ORM lib.
(faster than standard sql, not as fast as my optimizations :) )
Ill take that in a separate pull request whenever I have time

Small note to the framework.. The test have unique names, but they still
have an integer separating them in the result map. Maybe it would be
possible to use the name or classname as a hash instead. It took some
time before I understood why I got the exact same result in my optimized
and standard SQL, down to the millisecond.

/Mikael

On 24.04.2015 19:40, Dan J wrote:

This looks really good, the write times for raw SQL are hugely
improved (I saw a 2X to 3X improvement from a quick test run). A few
suggestions:

Subclass the existing raw SQL test classes with your optimized SQL
classes, |SQLite[Optimized]Executor extends SQLiteRawExecutor| and
|Message extends OptimizedMessage|. This will remove all the
duplicated code and help to highlight the differences in
implementation. You can also refactor writeWholeData() so that the
first half of the code is commonized between the two.
Add some comments explaining /why/ SQLite[Optimized]Executor is
faster. At the moment it takes quite a lot of code reading to
understand it, and it would be great to make the high level
explanation more clear.

I like the idea of keeping the raw SQL and Optimized SQL benchmarks
separate, to show how much faster you can make the writes if you do it
properly. :-)

If you aren't able to do the refactors then I might be able to find
some time to help do that, but I think you would be best placed to add
some comments. Thanks!


Reply to this email directly or view it on GitHub
#2 (comment).

@MazyNoc
Copy link
Author

MazyNoc commented May 11, 2015

Just looked at it again (much sooner than expected)
The tests are built up with singletons made from enums. This prohibit
the use of extends.
For the data objects (message and user) a lot of functions are static,
making them also not extendable without copying a lot of code.

in that light, I think it makes sense to keep them totally separated
since its easier to just look at one example (one package) to get all
code for a specific case without moving between the tests.

In production code I can see how the refactoring should have been done,
but this is a comparison and for some event tutorial code.

Let be know if you agree

/Mikael

On 24.04.2015 19:40, Dan J wrote:

This looks really good, the write times for raw SQL are hugely
improved (I saw a 2X to 3X improvement from a quick test run). A few
suggestions:

Subclass the existing raw SQL test classes with your optimized SQL
classes, |SQLite[Optimized]Executor extends SQLiteRawExecutor| and
|Message extends OptimizedMessage|. This will remove all the
duplicated code and help to highlight the differences in
implementation. You can also refactor writeWholeData() so that the
first half of the code is commonized between the two.
Add some comments explaining /why/ SQLite[Optimized]Executor is
faster. At the moment it takes quite a lot of code reading to
understand it, and it would be great to make the high level
explanation more clear.

I like the idea of keeping the raw SQL and Optimized SQL benchmarks
separate, to show how much faster you can make the writes if you do it
properly. :-)

If you aren't able to do the refactors then I might be able to find
some time to help do that, but I think you would be best placed to add
some comments. Thanks!


Reply to this email directly or view it on GitHub
#2 (comment).

daj added a commit that referenced this pull request May 13, 2015
New optimized raw sql executor
@daj daj merged commit c3fd32e into daj:master May 13, 2015
@daj
Copy link
Owner

daj commented May 13, 2015

I've merged and I will probably do some refactoring, so make sure you pull again before making further changes. Thanks!

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.

2 participants