| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
This reverts commit 796cab45561fce268aa74e6587cdb9cae3bb243e.
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
This reverts commit 393e65b4170608593ad82377a9eadc918e85698d and
ec51c3fedd16b561d096dcc1a6705fdc02ab7666
We don't want the records to hold hard references to transactions
because they point at records that have callbacks.
|
|
|
|
|
| |
this way we don't have to mutate a state object, we can just change the
state of the txn
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We only set the state on the record if that condition is `false` in the
first place, so we dont need to call that again. Also that call is
expensive, follow benchmark with before and after this change:
```
Calculating -------------------------------------
persisted? 15.272k i/100ms
-------------------------------------------------
persisted? 350.119k (± 4.6%) i/s - 1.756M
```
```
Calculating -------------------------------------
persisted? 25.988k i/100ms
-------------------------------------------------
persisted? 1.294M (± 5.3%) i/s - 6.445M
```
(benchmark borrowed from 57d35b2bf9e48173a5f97ccff5e6897f0c46411f)
|
|
|
|
|
| |
depth is always 0, so the index will always be false. No reason to
create the instance variable if it isn't used
|
| |
|
|
|
|
| |
[fixes #18903]
|
|
|
|
|
|
|
|
|
| |
Add after_commit_without_transaction_enrollment and
after_rollback_without_transaction_enrollment private callbacks so we
can create after_commit and after_rollback callbacks without having the
records automatic enrolled in the transaction.
[fixes #18904]
|
| |
|
|
|
|
|
| |
the transaction object shouldn't know so much about active record
objects, so let's push the conditionals in to the instance.
|
| |
|
|
|
|
| |
As discussed before, those methods should receive a keyword args instead of just parameters
|
|\
| |
| | |
Fix rollback of primarykey-less tables
|
| |
| |
| | |
If you have a table without a primary key, and an `after_commit` callback on that table (ie `has_transactional_callbacks?` returns true), then trying to rollback a transaction involving that record would result in “ActiveModel::MissingAttributeError: can't write unknown attribute ``”
|
|/
|
|
|
|
|
|
| |
Before this change any error raised inside a transaction callback
are rescued and printed in the logs.
Now these errors are not rescue anymore and just bubble up,
as the other callbacks.
|
|
|
|
|
|
|
|
|
|
| |
Before this commit, returning `false` in an ActiveRecord `before_` callback
such as `before_create` would halt the callback chain.
After this commit, the behavior is deprecated: will still work until
the next release of Rails but will also display a deprecation warning.
The preferred way to halt a callback chain is to explicitly `throw(:abort)`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit changes arguments and default value of CallbackChain's :terminator
option.
After this commit, Chains of callbacks defined **without** an explicit
`:terminator` option will be halted as soon as a `before_` callback throws
`:abort`.
Chains of callbacks defined **with** a `:terminator` option will maintain their
existing behavior of halting as soon as a `before_` callback matches the
terminator's expectation. For instance, ActiveModel's callbacks will still
halt the chain when a `before_` callback returns `false`.
|
|\ |
|
|/ |
|
|
|
|
| |
Add nodoc to some constants [skip ci]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The validation added in 5a3dc8092d19c816b0b1203945639cb91d065847 will
reject values for the `:on` option for after_commit and after_rollback
callbacks that are string values like `"create"`.
However, the error message says ":on conditions for after_commit and
after_rollback callbacks have to be one of create,destroy,update". That
looks like a string value *would* be valid.
This commit changes the error message to say ":on conditions for
after_commit and after_rollback callbacks have to be one of [:create,
:destroy, :update]", making it clearer that symbols are required.
|
|
|
|
| |
This will avoid naming clash with user defined methods
|
|
|
| |
The intention here is to make the required config copy-able from the console/logs, so add a newline at the end of the message to make that easier. (Otherwise it would be `... raise_in_transactional_callbacks = true (called from...`.)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Inspired by @tenderlove's work in
c363fff29f060e6a2effe1e4bb2c4dd4cd805d6e, this reduces the number of
strings allocated when running callbacks for ActiveRecord instances. I
measured that using this script:
```
require 'objspace'
require 'active_record'
require 'allocation_tracer'
ActiveRecord::Base.establish_connection adapter: "sqlite3",
database: ":memory:"
ActiveRecord::Base.connection.instance_eval do
create_table(:articles) { |t| t.string :name }
end
class Article < ActiveRecord::Base; end
a = Article.create name: "foo"
a = Article.find a.id
N = 10
result = ObjectSpace::AllocationTracer.trace do
N.times { Article.find a.id }
end
result.sort.each do |k,v|
p k => v
end
puts "total: #{result.values.map(&:first).inject(:+)}"
```
When I run this against master and this branch I get this output:
```
pete@balloon:~/projects/rails/activerecord$ git checkout master
M Gemfile
Switched to branch 'master'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_before
pete@balloon:~/projects/rails/activerecord$ git checkout remove-dynamic-send-on-built-in-callbacks
M Gemfile
Switched to branch 'remove-dynamic-send-on-built-in-callbacks'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_after
pete@balloon:~/projects/rails/activerecord$ diff allocations_before allocations_after
39d38
<
{["/home/pete/projects/rails/activesupport/lib/active_support/callbacks.rb",
81]=>[40, 0, 0, 0, 0, 0]}
42c41
< total: 630
---
> total: 590
```
In addition to this, there are two micro-optimizations present:
* Using `block.call if block` vs `yield if block_given?` when the block was being captured already.
```
pete@balloon:~/projects$ cat benchmark_block_call_vs_yield.rb
require 'benchmark/ips'
def block_capture_with_yield &block
yield if block_given?
end
def block_capture_with_call &block
block.call if block
end
def no_block_capture
yield if block_given?
end
Benchmark.ips do |b|
b.report("block_capture_with_yield") { block_capture_with_yield }
b.report("block_capture_with_call") { block_capture_with_call }
b.report("no_block_capture") { no_block_capture }
end
pete@balloon:~/projects$ ruby benchmark_block_call_vs_yield.rb
Calculating -------------------------------------
block_capture_with_yield
124979 i/100ms
block_capture_with_call
138340 i/100ms
no_block_capture 136827 i/100ms
-------------------------------------------------
block_capture_with_yield
5703108.9 (±2.4%) i/s - 28495212 in 4.999368s
block_capture_with_call
6840730.5 (±3.6%) i/s - 34169980 in 5.002649s
no_block_capture 5821141.4 (±2.8%) i/s - 29144151 in 5.010580s
```
* Defining and calling methods instead of using send.
```
pete@balloon:~/projects$ cat benchmark_method_call_vs_send.rb
require 'benchmark/ips'
class Foo
def tacos
nil
end
end
my_foo = Foo.new
Benchmark.ips do |b|
b.report('send') { my_foo.send('tacos') }
b.report('call') { my_foo.tacos }
end
pete@balloon:~/projects$ ruby benchmark_method_call_vs_send.rb
Calculating -------------------------------------
send 97736 i/100ms
call 151142 i/100ms
-------------------------------------------------
send 2683730.3 (±2.8%) i/s - 13487568 in 5.029763s
call 8005963.9 (±2.7%) i/s - 40052630 in 5.006604s
```
The result of this is making typical ActiveRecord operations slightly faster:
https://gist.github.com/phiggins/e46e51dcc7edb45b5f98
|
|
|
|
|
|
|
|
|
|
| |
Using heredoc would enforce line wrapping to whatever column width we decided to
use in the code, making it difficult for the users to read on some consoles.
This does make the source code read slightly worse and a bit more error-prone,
but this seems like a fair price to pay since the primary purpose for these
messages are for the users to read and the code will not stick around for too
long.
|
|
|
|
| |
[ci skip]
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, Active Record will rescue any errors raised within
after_rollback/after_create callbacks and print them to the
logs. Next versions of rails will not rescue those errors anymore,
and just bubble them up, as the other callbacks.
This adds a opt-in flag to enable that behaviour, of not rescuing
the errors.
Example:
# For not swallow errors in after_commit/after_rollback
config.active_record.errors_in_transactional_callbacks = true
[fixes #13460]
|
|\
| |
| |
| |
| |
| |
| | |
Remove dead branch when restoring ID within a transaction
Conflicts:
activerecord/lib/active_record/transactions.rb
|
| |
| |
| |
| |
| |
| |
| | |
There is no way to have an instance of an Active Record model where
`has_attribute?(self.class.primary_key)` returns false. The record is
always initialized in such a way that `@raw_attributes` will have an id
key with nil for the value.
|
|/
|
|
|
|
|
| |
Reduces the number of places that care about the internals of how we
store and type cast attributes. We do not need to go through the
dup/freeze dance, as you couldn't have saved a frozen new record anyway,
and that is the only time we would end up modifying the frozen hash.
|
|
|
|
|
|
|
|
|
| |
`@attributes` was actually used for `_before_type_cast` and friends,
while `@attributes_cache` is the type cast version (and caching is the
wrong word there, but I'm working on removing the conditionals around
that). I opted for `@raw_attributes`, because `_before_type_cast` is
also semantically misleading. The values in said hash are in the state
given by the form builder or database, so raw seemed to be a good word.
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
We are using that code path in only one place so we should not add a
conditional to all the other cases. This will avoid performance
regressions on the old paths.
|
| |
|
|
|
|
|
|
| |
Make sure when we clean the `@_start_transaction_state` var we do it in
the same code-path.
Also this makes `clear_transaction_record_state`, more consistent with `restore_transaction_record_state`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
rollback"
We are reverting these commits, because there are other caveats related
to dirty attributes not being restored when a transaction is rollback.
For instance, nested transactions cannot proper restore the dirty
attributes state after a rollback.
At the moment, the dirty attributes are scoped by the transaction.
When we call `.save` on a record, the dirty attributes will be reset even
if the transaction gets rollback.
[related #13166]
[related #15018]
[related #15016]
[closes #15019]
This reverts commits
* bab48f0a3d53a08bc23ea0887219e8deb963c3d9
* b0fa7cf3ff8432cc2eef3682b34763b7f8c93cc8.
* 73fb39b6faa9de593ae75ad4e3b8e065ea0e53af
* 37c238927fbed059de3f26a90d8923fb377568a5.
* 8d8d4f1560264cd1c74364d67fa0501f6dd2c4fa
Revert "Merge pull request #13166 from bogdan/transaction-magic"
|
| |
|
|
|
|
| |
[related #13166]
|
|\
| |
| |
| | |
[Regression 3.2 -> 4.0] Fix bugs with changed attributes tracking when transaction gets rollback
|
| | |
|
| |
| |
| |
| | |
custom primary_key that didn't save due to validation error
|
| |
| |
| |
| | |
[fixes #12566]
|