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

Use execute_batch2 rather than execute_batch to fix performance regression for fixture loading #35844

Merged

Conversation

@kamipo
Copy link
Member

@kamipo kamipo commented Apr 3, 2019

d8d6bd5 makes fixture loading to bulk statements by using
execute_batch for sqlite3 adapter. But execute_batch is slower and
it caused the performance regression for fixture loading.

In sqlite3 1.4.0, it have new batch method execute_batch2. I've
confirmed execute_batch2 is extremely faster than execute_batch.
So I think it is worth to upgrade sqlite3 to 1.4.0 to use that method.

Before:

% ARCONN=sqlite3 bundle exec ruby -w -Itest test/cases/associations/eager_test.rb -n test_eager_loading_too_may_ids
Using sqlite3
Run options: -n test_eager_loading_too_may_ids --seed 35790

# Running:

.

Finished in 202.437406s, 0.0049 runs/s, 0.0049 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
ARCONN=sqlite3 bundle exec ruby -w -Itest  -n test_eager_loading_too_may_ids  142.57s user 60.83s system 98% cpu 3:27.08 total

After:

% ARCONN=sqlite3 bundle exec ruby -w -Itest test/cases/associations/eager_test.rb -n test_eager_loading_too_may_ids
Using sqlite3
Run options: -n test_eager_loading_too_may_ids --seed 16649

# Running:

.

Finished in 8.471032s, 0.1180 runs/s, 0.1180 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
ARCONN=sqlite3 bundle exec ruby -w -Itest  -n test_eager_loading_too_may_ids  10.71s user 1.36s system 95% cpu 12.672 total

What do you think?

…egression for fixture loading

d8d6bd5 makes fixture loading to bulk statements by using
`execute_batch` for sqlite3 adapter. But `execute_batch` is slower and
it caused the performance regression for fixture loading.

In sqlite3 1.4.0, it have new batch method `execute_batch2`. I've
confirmed `execute_batch2` is extremely faster than `execute_batch`.
So I think it is worth to upgrade sqlite3 to 1.4.0 to use that method.

Before:

```
% ARCONN=sqlite3 bundle exec ruby -w -Itest test/cases/associations/eager_test.rb -n test_eager_loading_too_may_ids
Using sqlite3
Run options: -n test_eager_loading_too_may_ids --seed 35790

# Running:

.

Finished in 202.437406s, 0.0049 runs/s, 0.0049 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
ARCONN=sqlite3 bundle exec ruby -w -Itest  -n test_eager_loading_too_may_ids  142.57s user 60.83s system 98% cpu 3:27.08 total
```

After:

```
% ARCONN=sqlite3 bundle exec ruby -w -Itest test/cases/associations/eager_test.rb -n test_eager_loading_too_may_ids
Using sqlite3
Run options: -n test_eager_loading_too_may_ids --seed 16649

# Running:

.

Finished in 8.471032s, 0.1180 runs/s, 0.1180 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
ARCONN=sqlite3 bundle exec ruby -w -Itest  -n test_eager_loading_too_may_ids  10.71s user 1.36s system 95% cpu 12.672 total
```
Copy link
Member

@eileencodes eileencodes left a comment

Thanks for the quick fix ❤️

kaspth
kaspth approved these changes Apr 3, 2019
Copy link
Member

@kaspth kaspth left a comment

Looks like the build passed too!

@kamipo kamipo merged commit 9bccf46 into rails:master Apr 3, 2019
2 of 3 checks passed
@kamipo kamipo deleted the fix_fixture_loading_performance_regression branch Apr 3, 2019
@@ -10,7 +10,7 @@
require "active_record/connection_adapters/sqlite3/schema_dumper"
require "active_record/connection_adapters/sqlite3/schema_statements"

gem "sqlite3", "~> 1.3", ">= 1.3.6"
gem "sqlite3", "~> 1.4"
Copy link
Contributor

@bogdanvlviv bogdanvlviv Apr 6, 2019

We might need to add changelog entry to note that Active Record is compatible with sqlite3 ~> 1.4 since Rails 6.0.

Agreed. This has just caught us out in cucumber-rails as we were using a 1.3 version of sqllite across the board. We'll put a conditional in for rails 6.

mracos added a commit to heartcombo/devise that referenced this issue May 4, 2019
Also bumped sqlite from 1.3.6 to 1.4 because besides conflicting with
the version that the sqlite adapter was trying to load [0], it is supported
officially since rails 6 [1].

Related:
[0] rails/rails#35153
[1] rails/rails#35844
mracos added a commit to heartcombo/devise that referenced this issue May 4, 2019
Also bumped sqlite from 1.3.6 to 1.4 because besides conflicting with
the version that the sqlite adapter was trying to load [0], it is supported
officially since rails 6 [1].

Related:
[0] rails/rails#35153
[1] rails/rails#35844
y-yagi added a commit to y-yagi/enumerize that referenced this issue May 25, 2019
Also, moved sqlite3 gem to individual gemfiles because Rails 6 requires
sqlite3 upper 1.4.0.
Ref: rails/rails#35844
jtapia added a commit to jtapia/devise that referenced this issue Jun 6, 2019
Also bumped sqlite from 1.3.6 to 1.4 because besides conflicting with
the version that the sqlite adapter was trying to load [0], it is supported
officially since rails 6 [1].

Related:
[0] rails/rails#35153
[1] rails/rails#35844
mygreatstory pushed a commit to mygreatstory/rubenumerize that referenced this issue Jan 9, 2021
Also, moved sqlite3 gem to individual gemfiles because Rails 6 requires
sqlite3 upper 1.4.0.
Ref: rails/rails#35844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants