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

Another array allocation reduction #2020

Merged

Conversation

ericproulx
Copy link
Contributor

I found another place where we can reduce array allocation.

# current

Total allocated: 31701608 bytes (165916 objects)
Total retained:  3411991 bytes (8910 objects)

allocated memory by class
-----------------------------------
  15730608  Hash
   6726012  String
   5870664  Array
   3079724  Regexp

# this PR
Total allocated: 31103301 bytes (151618 objects)
Total retained:  3410317 bytes (8901 objects)

allocated memory by class
-----------------------------------
  15728880  Hash
   6725106  String
   5275928  Array
   3078867  Regexp

@grape-bot
Copy link

grape-bot commented Mar 17, 2020

1 Warning
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.

Generated by 🚫 danger

Convert literal array in CONST

Replace =~ or match by match? when we don't need the MatchData object. match? function is new from 2.4

Add CHANGELOG.md

Add SafeOperator before match

Add upgrade notice for Array[String]

If Array[String] is used without a coerce_with block, existing APIs may
quietly fail without explanation. This change to the upgrade
documentation makes that clear.

Closes ruby-grape#2013

Add upgrade notice on upgrading all Array types

Previously Vitrus implicitly coerced strings to integers when
Array[Integer] types were used. With dry-types, this is no longer done,
and APIs that expected this behavior will fail with an invalid type
check.  Expand the upgrade docs to include all Array types and an
example for Array[Integer].

Add CHANGELOG.md
@ericproulx ericproulx force-pushed the another_array_allocation_reduction branch from 7d764ff to da75f7b Compare March 17, 2020 10:02
@@ -2,6 +2,7 @@

#### Features
* Your contribution here.
* [#2020](https://github.com/ruby-grape/grape/pull/2020): Reduce array allocation - [@ericproulx](https://github.com/ericproulx).
Copy link
Member

Choose a reason for hiding this comment

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

For next time, you can do [#2020](https://github.com/ruby-grape/grape/pull/2020), [#2014](https://github.com/ruby-grape/grape/pull/2014): Reduce ....

@dblock
Copy link
Member

dblock commented Mar 17, 2020

Minor. For next time, try amending your commit (git add ; git commit --amend) and force pushing the branch, so we end up with a neat single commit in these PRs :)

@dblock dblock merged commit 0e05af7 into ruby-grape:master Mar 17, 2020
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