| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
| |
| |
| | |
There was a typo in the variable name leading to infinite recursion
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We had previously updated this to attempt to map over whatever was
passed in, so that additional types like range and array could benefit
from this behavior without the time zone converter having to deal with
every known type.
However, the default behavior of a type is to just yield the given value
to `map`, which means that if we don't actually know how to handle a
value, we'll just recurse infinitely. Since both uses of `map` in this
case occur in cases where we know receiving the same object will
recurse, we can just break on reference equality.
Fixes #23241.
|
|
|
|
|
|
| |
This is really strange, as it passes on Travis for 2.3, and it passes
for me locally on 2.2. But on travis w/ 2.2, passing `Float::INFINITY`
in is resulting in an infinite loop. This should resolve it.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is an alternate implementation to #22875, that generalizes a lot of
the logic that type decorators are going to need, in order to have them
work with arrays, ranges, etc. The types have the ability to map over a
value, with the default implementation being to just yield that given
value. Array and Range give more appropriate definitions.
This does not automatically make ranges time zone aware, as they need to
be added to the `time_zone_aware` types config, but we could certainly
make that change if we feel it is appropriate. I do think this would be
a breaking change however, and should at least have a deprecation cycle.
Closes #22875.
/cc @matthewd
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
```
# before
DEPRECATION WARNING: Time columns will become time zone aware in Rails 5.1. This
still causes `String`s to be parsed as if they were in `Time.zone`,
and `Time`s to be converted to `Time.zone`.
To keep the old behavior, you must add the following to your initializer:
config.active_record.time_zone_aware_types = [:datetime]
To silence this deprecation warning, add the following:
config.active_record.time_zone_aware_types << :time
```
```
# after
DEPRECATION WARNING: Time columns will become time zone aware in Rails 5.1. This
still causes `String`s to be parsed as if they were in `Time.zone`,
and `Time`s to be converted to `Time.zone`.
To keep the old behavior, you must add the following to your initializer:
config.active_record.time_zone_aware_types = [:datetime]
To silence this deprecation warning, add the following:
config.active_record.time_zone_aware_types << :time
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The focus of this change is to make the API more accessible.
References to method and classes should be linked to make it easy to
navigate around.
This patch makes exzessiv use of `rdoc-ref:` to provide more readable
docs. This makes it possible to document `ActiveRecord::Base#save` even
though the method is within a separate module
`ActiveRecord::Persistence`. The goal here is to bring the API closer to
the actual code that you would write.
This commit only deals with Active Record. The other gems will be
updated accordingly but in different commits. The pass through Active
Record is not completely finished yet. A follow up commit will change
the spots I haven't yet had the time to update.
/cc @fxn
|
|
|
|
| |
to reduce allocation of same object
|
|
|
|
|
| |
For reads, we never need to construct this object. The double `defined?`
check is to avoid errors in tests
|
|
|
|
|
| |
There were a few places where I missed a `create` vs `new`
before_type_cast check, and the semantics of `reload` became wrong.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We can skip the allocation of a full `AttributeSet` by changing the
semantics of how we structure things. Instead of comparing two separate
`AttributeSet` objects, and `Attribute` is now a singly linked list of
every change that has happened to it. Since the attribute objects are
immutable, to apply the changes we simply need to copy the head of the
list.
It's worth noting that this causes one subtle change in the behavior of
AR. When a record is saved successfully, the `before_type_cast` version
of everything will be what was sent to the database. I honestly think
these semantics make more sense, as we could have just as easily had the
DB do `RETURNING *` and updated the record with those if we had things
like timestamps implemented at the DB layer.
This brings our performance closer to 4.2, but we're still not quite
there.
|
|
|
|
|
|
|
|
|
|
|
| |
I'm looking to move towards a tree-like structure for dirty checking
that involves an attribute holding onto the attribute that it was
created from. This means that `changed?` can be fully encapsulated on
that object. Since the objects are immutable, in `changes_applied`, we
can simply perform a shallow dup, instead of a deep one.
I'm not sure if that will actually end up in a performance boost, but
I'd like to semantically separate these concepts regardless
|
|
|
|
|
|
|
|
|
|
|
| |
When a time object was assigned to a datetime column, the decorator for
TZ aware attributes wouldn't call super, so when using a database
without support for subsecond precision, the nanosecond would not be
truncated, leading to the value being marked as changed.
Interestingly, this also shows our new implementation of dirty checking
to be more robust than the old one (with less code and better
performance! :tada:!!!)
|
|
|
|
|
|
| |
When I removed the call to `super` to avoid the setting of
`@previous_changes`, I forgot to duplicate the other part of that
behavior, which led to failing tests
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The biggest source of the performance regression in these methods
occurred because dirty tracking required eagerly materializing and type
casting the assigned values. In the previous commits, I've changed dirty
tracking to perform the comparisons lazily. However, all of this is moot
when calling `save`, since `changes_applied` will be called, which just
ends up eagerly materializing everything, anyway. With the new mutation
tracker, it's easy to just compare the previous two hashes in the same
lazy fashion.
We will not have aliasing issues with this setup, which is proven by the
fact that we're able to detect nested mutation.
Before:
User.create! 2.007k (± 7.1%) i/s - 10.098k
After:
User.create! 2.557k (± 3.5%) i/s - 12.789k
Fixes #19859
|
|
|
|
|
|
|
|
|
| |
In order to improve the performance of dirty checking, we're going to
need to duplicate all of the `previous_` methods in Active Model.
However, these methods are basically the same as their non-previous
counterparts, but comparing `@original_attributes` to
`@previous_original_attributes` instead of `@attributes` and
`@original_attributes`. This will help reduce that duplication.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This moves a bit more of the logic required for dirty checking into the
attribute objects. I had hoped to remove the `with_value_from_database`
stuff, but unfortunately just calling `dup` on the attribute objects
isn't enough, since the values might contain deeply nested data
structures. I think this can be cleaned up further.
This makes most dirty checking become lazy, and reduces the number of
object allocations and amount of CPU time when assigning a value. This
opens the door (but doesn't quite finish) to improving the performance
of writes to a place comparable to 4.1
|
|
|
|
| |
The first step of bringing typecasting to ActiveModel
|
|\
| |
| | |
nodoc raw_write_attribute
|
| |
| |
| |
| |
| | |
Is this supposed to be public API? If so, I can document it instead.
:memo:
|
| |
| |
| |
| |
| |
| |
| | |
We are only supporting Ruby 2.2 and later in Rails 5, so we do not need
an actual constant here. Additionally, referencing a constant actually
does a hash lookup (because constants are not constant in Ruby >_>).
This will be marginally (likely immeasurable) faster. It is less ugly.
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I wrote a utility that helps find areas where you could optimize your program using a frozen string instead of a string literal, it's called [let_it_go](https://github.com/schneems/let_it_go). After going through the output and adding `.freeze` I was able to eliminate the creation of 1,114 string objects on EVERY request to [codetriage](codetriage.com). How does this impact execution?
To look at memory:
```ruby
require 'get_process_mem'
mem = GetProcessMem.new
GC.start
GC.disable
1_114.times { " " }
before = mem.mb
after = mem.mb
GC.enable
puts "Diff: #{after - before} mb"
```
Creating 1,114 string objects results in `Diff: 0.03125 mb` of RAM allocated on every request. Or 1mb every 32 requests.
To look at raw speed:
```ruby
require 'benchmark/ips'
number_of_objects_reduced = 1_114
Benchmark.ips do |x|
x.report("freeze") { number_of_objects_reduced.times { " ".freeze } }
x.report("no-freeze") { number_of_objects_reduced.times { " " } }
end
```
We get the results
```
Calculating -------------------------------------
freeze 1.428k i/100ms
no-freeze 609.000 i/100ms
-------------------------------------------------
freeze 14.363k (± 8.5%) i/s - 71.400k
no-freeze 6.084k (± 8.1%) i/s - 30.450k
```
Now we can do some maths:
```ruby
ips = 6_226k # iterations / 1 second
call_time_before = 1.0 / ips # seconds per iteration
ips = 15_254 # iterations / 1 second
call_time_after = 1.0 / ips # seconds per iteration
diff = call_time_before - call_time_after
number_of_objects_reduced * diff * 100
# => 0.4530373333993266 miliseconds saved per request
```
So we're shaving off 1 second of execution time for every 220 requests.
Is this going to be an insane speed boost to any Rails app: nope. Should we merge it: yep.
p.s. If you know of a method call that doesn't modify a string input such as [String#gsub](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37) please [give me a pull request to the appropriate file](https://github.com/schneems/let_it_go/blob/b0e2da69f0cca87ab581022baa43291cdf48638c/lib/let_it_go/core_ext/string.rb#L37), or open an issue in LetItGo so we can track and freeze more strings.
Keep those strings Frozen
![](https://www.dropbox.com/s/z4dj9fdsv213r4v/let-it-go.gif?dl=1)
|
| |
|
|
|
| |
the test case for this commit is in cd3f5db
|
|
|
|
|
|
|
|
|
|
|
| |
If a getter has side effects on the DB, `changes_applied` will be called
twice. The second time will try and remove the changed attributes cache,
and will crash because it's already been unset. This also demonstrates
that we shouldn't assume that calling getters won't change the value of
`changed_attributes`, and we need to clear the cache if an attribute is
modified.
Fixes #20531.
|
|\
| |
| |
| |
| |
| | |
Update docs for ActiveRecord `serialize`
[ci skip]
|
|/
|
|
|
|
|
|
|
|
|
|
| |
For certain column types, using `serialize` is unnecessary, or the user
may get unexpected contents back from the DB adapter (which is handling
some basic deserialization for them). Call this out in the
documentation.
For background, see:
https://gist.github.com/ernie/33f75f2294885b9806f9
https://twitter.com/erniemiller/status/604262907442905090
|
| |
|
| |
|
|
|
|
|
| |
This predicate is only used in `query_attribute`, and is relatively easy
to remove without adding a bunch of is a checks.
|
|
|
|
|
|
|
| |
It only existed to make sure the subclasses of `Delegator` were YAML
serializable. As of Ruby 2.2, these are YAML dumpable by default, as it
includes
https://github.com/tenderlove/psych/commit/2a4d9568f7d5d19c00231cf48eb855cc45ec3394
|
|
|
|
|
|
|
|
|
|
|
|
| |
This allows us to remove `Type::Value#klass`, as it was only used for
multi-parameter assignment to reach into the types internals. The
relevant type objects now accept a hash in addition to their previous
accepted arguments to `type_cast_from_user`. This required minor
modifications to the tests, since previously they were relying on the
fact that mulit-parameter assignement was reaching into the internals of
time zone aware attributes. In reaility, changing those properties at
runtime wouldn't change the accessor methods for all other forms of
assignment.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The `&block` form is more than twice as fast as the manual form of
delegation (and is the code I'd rather write anyway). Unfortunately,
it's still twice as slow on MRI. However, this is enough of a hotspot to
justify giving JRuby special treatment.
I can't currently provide benchmarks in the context of Active Record,
since the JDBC adapters still aren't updated for 4.2, but the actual
work performed (assuming it's been read at least once already) will have
nearly the same performance characteristics as
https://gist.github.com/sgrif/b86832786551aaee74de.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It's finally finished!!!!!!! The reason the Attributes API was kept
private in 4.2 was due to some publicly visible implementation details.
It was previously implemented by overloading `columns` and
`columns_hash`, to make them return column objects which were modified
with the attribute information.
This meant that those methods LIED! We didn't change the database
schema. We changed the attribute information on the class. That is
wrong! It should be the other way around, where schema loading just
calls the attributes API for you. And now it does!
Yes, this means that there is nothing that happens in automatic schema
loading that you couldn't manually do yourself. (There's still some
funky cases where we hit the connection adapter that I need to handle,
before we can turn off automatic schema detection entirely.)
There were a few weird test failures caused by this that had to be
fixed. The main source came from the fact that the attribute methods are
now defined in terms of `attribute_names`, which has a clause like
`return [] unless table_exists?`. I don't *think* this is an issue,
since the only place this caused failures were in a fake adapter which
didn't override `table_exists?`.
Additionally, there were a few cases where tests were failing because a
migration was run, but the model was not reloaded. I'm not sure why
these started failing from this change, I might need to clear an
additional cache in `reload_schema_from_cache`. Again, since this is not
normal usage, and it's expected that `reset_column_information` will be
called after the table is modified, I don't think it's a problem.
Still, test failures that were unrelated to the change are worrying, and
I need to dig into them further.
Finally, I spent a lot of time debugging issues with the mutex used in
`define_attribute_methods`. I think we can just remove that method
entirely, and define the attribute methods *manually* in the call to
`define_attribute`, which would simplify the code *tremendously*.
Ok. now to make this damn thing public, and work on moving it up to
Active Model.
|
|
|
|
|
|
| |
Remaining are `limit`, `precision`, `scale`, and `type` (the symbol
version). These will remain on the column, since they mirror the options
to the `column` method in the schema definition DSL
|
|\
| |
| | |
[ci skip] fix typo still -> will
|
| | |
|
|/
|
|
| |
Fixes #18580.
|
|
|
|
|
|
|
|
|
|
|
| |
When an attribute is assigned, we determine if it was already marked as
changed so we can determine if we need to clear the changes, or mark it
as changed. Since this only affects the `attributes_changed_by_setter`
hash, in-place changes are irrelevant to this process. Since calculating
in-place changes can be expensive, we can just skip it here.
I also added a test for the only edge case I could think of that would
be affected by this change.
|
| |
|
|
|
|
|
|
| |
The types that are affected by `time_zone_aware_attributes` (which is on
by default) have been made configurable, in case this is a breaking
change for existing applications.
|
|
|
|
|
|
| |
While we don't want to change the form input when validations fail,
blindly using `_before_type_cast` will cause the input to display the
wrong data for any type which does additional work on database values.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This sets a precident for how we handle `attribute` calls, which aren't
backed by a database column. We should not take this as a conscious
decision on how to handle them, and this can change when we make
`attribute` public if we have better ideas in the future.
As the composed attributes API gets fleshed out, I expect the
`persistable_attributes` method to change to
`@attributes.select(&:persistable).keys`, or some more performant
variant there-of. This can probably go away completely once we fully
move dirty checking into the attribute objects once it gets moved up to
Active Model.
Fixes #18407
|
|
|
|
| |
ref #52f641264b1325a4c2bdce7971b14524bd4905f1
|
| |
|
|
|
|
|
| |
Conflicts:
activerecord/lib/active_record/attribute_methods/read.rb
|
|
|
|
| |
These requires were added only to change deprecation message
|
| |
|
| |
|
| |
|