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.

Sign up for GitHub

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

Jump to bottom

Enhance marshaling performances #339

Merged
merged 9 commits into from
May 31, 2019
Merged

Enhance marshaling performances #339

merged 9 commits into from
May 31, 2019

Conversation

macisamuele
Copy link
Collaborator

@macisamuele macisamuele commented May 27, 2019 •
edited
Loading

This PR mimics what has been done in #336 for the unmarshaling process.
As for the unmarshaling also in case of the marshaling process we have a good performance improvements.

Raw data: https://gist.github.com/macisamuele/ee3b66176becab678aedf943e7a0e512

  • the worst performance improvement is ~61% (if in the branch takes 1 then on master takes 2.58)
  • the best performance improvement is ~72% (if in the branch takes 1 then on master takes 3.60)
  • the average performance improvement is ~79% (if the branch takes 1 then on master takes 4.89)

@coveralls
Copy link

coveralls commented May 27, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.4%) to 97.382% when pulling 8e30cda on macisamuele:maci-enhance-marshaling-performances into 665ca96 on Yelp:master.

macisamuele
macisamuele commented May 28, 2019
View reviewed changes
Copy link
Collaborator Author

@macisamuele macisamuele left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Docstrings are not accurate, I'll be updating them soon

sjaensch
sjaensch reviewed May 28, 2019
View reviewed changes
bravado_core/_decorators.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
@macisamuele macisamuele force-pushed the maci-enhance-marshaling-performances branch from 069b71f to 0811f99 Compare May 28, 2019 08:03
@macisamuele macisamuele force-pushed the maci-enhance-marshaling-performances branch from 0811f99 to f024b4a Compare May 28, 2019 08:05
@macisamuele macisamuele force-pushed the maci-enhance-marshaling-performances branch from e2cc20e to 2d0b416 Compare May 28, 2019 12:36
This was referenced May 29, 2019
Merged
Merged
sjaensch
sjaensch reviewed May 31, 2019
View reviewed changes
Copy link
Contributor

@sjaensch sjaensch left a comment

Choose a reason for hiding this comment

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

Can you post updated performance data now that the PR looks to be ready for merging?

bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/marshal.py Outdated Show resolved Hide resolved
bravado_core/_decorators.py Show resolved Hide resolved
@macisamuele
Copy link
Collaborator Author

@sjaensch I've updated the gist content and the description.
Performance improvements (according to the tests/profiling/marshal_profiler_test.py) are bound between 61% and 79%

@macisamuele macisamuele merged commit 411bdbb into Yelp:master May 31, 2019
@macisamuele macisamuele deleted the maci-enhance-marshaling-performances branch May 31, 2019 14:08
@macisamuele macisamuele mentioned this pull request May 31, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjaensch sjaensch sjaensch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@macisamuele @coveralls @sjaensch