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

Swap out KeyTransform for CaseTransform #1993

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Dec 4, 2016

Purpose

To allow key transform to be swapped out with native extensions for performance boosts. See: https://github.com/NullVoxPopuli/case_transform-rust-extensions (Up to 28 times faster)

Changes

  • Delete KeyTransform
  • Use CaseTransform Gem

Caveats

None

Related GitHub issues

#1834?

Additional helpful information

Benchmarks (Ruby only):

This PR:

camel 8637.508094079541/ips; 189 objects
camel_lower 8407.325157949488/ips; 189 objects
dash 6564.254917142684/ips; 189 objects
unaltered 7704142.353092194/ips; 1 objects
underscore 6792.223670331557/ips; 189 objects

Master:

camel 644.5149854118894/ips; 3653 objects
camel_lower 729.6670949346017/ips; 2622 objects
dash 3320.993361425401/ips; 770 objects
unaltered 7361102.918352639/ips; 1 objects
underscore 6463.657823115512/ips; 313 objects

The performance boost here is due to CaseTransform utilizing a cache.

@mention-bot
Copy link

@NullVoxPopuli, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dubadub, @remear and @iancanderson to be potential reviewers.

@@ -43,6 +43,7 @@ Gem::Specification.new do |spec|
# 'thread_safe'

spec.add_runtime_dependency 'jsonapi', '0.1.1.beta2'
spec.add_runtime_dependency 'case_transform', '>= 0.2'
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see this moved to the rails-api organization first. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm cool with that.
should we email github to just assign the existing repo to rails-api?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@NullVoxPopuli NullVoxPopuli force-pushed the swap-out-key-transform-with-case-transform branch from b77fa93 to e222aac Compare December 7, 2016 15:35
@NullVoxPopuli
Copy link
Contributor Author

rebased

@NullVoxPopuli NullVoxPopuli merged commit 0422a1e into rails-api:master Dec 7, 2016
@NullVoxPopuli NullVoxPopuli deleted the swap-out-key-transform-with-case-transform branch December 7, 2016 15:49
bf4 pushed a commit that referenced this pull request Jan 6, 2017
* delete KeyTransform, use CaseTransform

* added changelog

Conflicts:
	CHANGELOG.md
bf4 pushed a commit that referenced this pull request Jan 6, 2017
* delete KeyTransform, use CaseTransform

* added changelog

Conflicts:
	CHANGELOG.md
bf4 added a commit that referenced this pull request Jan 10, 2017
* Merge pull request #1990 from mxie/mx-result-typo

Fix typos and capitalization in Relationship Links docs [ci skip]

* Merge pull request #1992 from ojiry/bump_ruby_versions

Run tests by Ruby 2.2.6 and 2.3.3

* Merge pull request #1994 from bf4/promote_architecture

Promote important architecture description that answers a lot of questions we get
Conflicts:
	docs/ARCHITECTURE.md

* Merge pull request #1999 from bf4/typos

Fix typos [ci skip]

* Merge pull request #2000 from berfarah/patch-1

Link to 0.10.3 tag instead of `master` branch

* Merge pull request #2007 from bf4/check_ci

Test was failing due to change in JSON exception message when parsing empty string

* Swap out KeyTransform for CaseTransform (#1993)

* delete KeyTransform, use CaseTransform

* added changelog

Conflicts:
	CHANGELOG.md

* Merge pull request #2005 from kofronpi/support-ruby-2.4

Update jsonapi runtime dependency to 0.1.1.beta6

* Bump to v0.10.4

* Merge pull request #2018 from rails-api/bump_version

Bump to v0.10.4 [ci skip]
Conflicts:
	CHANGELOG.md

* Merge pull request #2019 from bf4/fix_method_redefined_warning

Fix AMS warnings

* Merge pull request #2020 from bf4/silence_grape_warnings

Silence Grape warnings

* Merge pull request #2017 from bf4/remove_warnings

Fix mt6 assert_nil warnings

* Updated isolated tests to assert correct behavior. (#2010)

* Updated isolated tests to assert correct behavior.
* Added check to get unsafe params if rails version is great than 5

* Merge pull request #2012 from bf4/cleanup_isolated_jsonapi_renderer_tests_a_bit

Cleanup assertions in isolated jsonapi renderer tests a bit

* Add Model#attributes helper; make test attributes explicit

* Fix model attributes accessors

* Fix typos

* Randomize testing of compatibility layer against regressions

* Test bugfix

* Add CHANGELOG

* Merge pull request #1981 from groyoh/link_doc

Fix relationship links doc
Conflicts:
	CHANGELOG.md
GregPK pushed a commit to GregPK/active_model_serializers that referenced this pull request Apr 25, 2017
* delete KeyTransform, use CaseTransform

* added changelog
GregPK pushed a commit to GregPK/active_model_serializers that referenced this pull request Apr 25, 2017
* Merge pull request rails-api#1990 from mxie/mx-result-typo

Fix typos and capitalization in Relationship Links docs [ci skip]

* Merge pull request rails-api#1992 from ojiry/bump_ruby_versions

Run tests by Ruby 2.2.6 and 2.3.3

* Merge pull request rails-api#1994 from bf4/promote_architecture

Promote important architecture description that answers a lot of questions we get
Conflicts:
	docs/ARCHITECTURE.md

* Merge pull request rails-api#1999 from bf4/typos

Fix typos [ci skip]

* Merge pull request rails-api#2000 from berfarah/patch-1

Link to 0.10.3 tag instead of `master` branch

* Merge pull request rails-api#2007 from bf4/check_ci

Test was failing due to change in JSON exception message when parsing empty string

* Swap out KeyTransform for CaseTransform (rails-api#1993)

* delete KeyTransform, use CaseTransform

* added changelog

Conflicts:
	CHANGELOG.md

* Merge pull request rails-api#2005 from kofronpi/support-ruby-2.4

Update jsonapi runtime dependency to 0.1.1.beta6

* Bump to v0.10.4

* Merge pull request rails-api#2018 from rails-api/bump_version

Bump to v0.10.4 [ci skip]
Conflicts:
	CHANGELOG.md

* Merge pull request rails-api#2019 from bf4/fix_method_redefined_warning

Fix AMS warnings

* Merge pull request rails-api#2020 from bf4/silence_grape_warnings

Silence Grape warnings

* Merge pull request rails-api#2017 from bf4/remove_warnings

Fix mt6 assert_nil warnings

* Updated isolated tests to assert correct behavior. (rails-api#2010)

* Updated isolated tests to assert correct behavior.
* Added check to get unsafe params if rails version is great than 5

* Merge pull request rails-api#2012 from bf4/cleanup_isolated_jsonapi_renderer_tests_a_bit

Cleanup assertions in isolated jsonapi renderer tests a bit

* Add Model#attributes helper; make test attributes explicit

* Fix model attributes accessors

* Fix typos

* Randomize testing of compatibility layer against regressions

* Test bugfix

* Add CHANGELOG

* Merge pull request rails-api#1981 from groyoh/link_doc

Fix relationship links doc
Conflicts:
	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants