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

Stop serializing and parsing columns_hash in Active Record schema caches #36518

Merged
merged 1 commit into from Jun 19, 2019

Conversation

Projects
None yet
3 participants
@casperisfine
Copy link

commented Jun 19, 2019

Followup: #35891

As discussed here: #35891 (comment)

columns_hash is simply columns indexed by name. So serializing / deserializing it is much slower than simply reindexing the columns from the columns array.

NB: This changes the serialization format. Not sure if it's important or not. I modified the patch so that is doesn't cause format compatibility issues.

@kaspth
cc @rafaelfranca @Edouard-chin

@rails-bot rails-bot bot added the activerecord label Jun 19, 2019

@casperisfine casperisfine force-pushed the Shopify:drop-schema-cache-column-hash branch from 8edb1c7 to dc99e5f Jun 19, 2019

@kaspth kaspth merged commit 8a20a40 into rails:master Jun 19, 2019

2 checks passed

buildkite/rails Build #61824 passed (9 minutes, 31 seconds)
Details
codeclimate All good!
Details
@kaspth

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

Just to reexplain it to myself: it's slow because the YAML for columns is essentially duplicated for columns_hash except there's an extra layer of nesting for the column name keys. Which slows serializing and parsing the YAML. So since columns has the data we want it's far less expensive to just run the calculation in Ruby.

@kaspth

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

Thanks for working on this! Really cool ❤️

@casperisfine

This comment has been minimized.

Copy link
Author

commented Jun 19, 2019

Yeah, it's not that this hash is particularly slow to parse, but more that it's much slower than simply reindexing the columns we already parsed once.

It also saves us from having to deduplicate the column objects a second time. It's win-win.

@casperisfine casperisfine deleted the Shopify:drop-schema-cache-column-hash branch Jun 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.