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

Store MIME parents in a distinct Hash #72

Merged
merged 1 commit into from
May 10, 2022

Conversation

casperisfine
Copy link

Since most entries don't have a parent, this
allows to remove one array per entry, saving about 50KiB (~10%).

Before:

Total retained: 506.64 kB (8558 objects)

After:

Total retained: 450.85 kB (7006 objects)

cc @gmcgibbon (let me know if I'm pushing this a bit too far)

Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if I'm pushing this a bit too far

I think the memory savings are marginal, but still worth it. Especially if the DB were to grow (via custom types etc.) this is a more efficient way to store the data. I find this format easier to reason about too, so I'll approve based on that.

Also regarding the build, considering we're still testing Ruby 2.2 on CI, we can probably bump the required ruby version. Rails requires 2.7 and any app still using 2.2 should really be upgrading their app. Let's fix that before merging this.

@casperisfine
Copy link
Author

we can probably bump the required ruby version

Hum, it's very unclear to me how the buildkite pipelined is defined. It's not in the repo, so I don't know how to remove Ruby 2.2 from the list of tested ruby. @matthewd would you know?

Also I wonder if marcel really benefit from buildkite over regular GitHub Actions.

@gmcgibbon
Copy link
Member

https://github.com/rails/gem-buildkite-config

The config is generated. If you bump the required ruby it should "just work" I believe.

@casperisfine
Copy link
Author

Oh TIL, that's really neat.

Since most entries don't have a parent, this
allows to remove one array per entry, saving about 50KiB (~10%).
@byroot byroot merged commit e4896ee into rails:main May 10, 2022
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.

3 participants