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

already loading field on define and should not need to calcualte the … #295

Merged
merged 4 commits into from
Feb 6, 2016

Conversation

abrandoned
Copy link
Contributor

…tag encoding at runtime

@film42 per our discussion on knowing the tag Varint at load time; we can store the encoded tag value on the field object in the definition and read it directly during the serialization phase instead of calculating the varint again

removes ~1.5 seconds on 15 second 100_000 object serialize

@film42

else
fail TypeError, "Unacceptable value #{val} for field #{field.name} of type #{field.type_class}"
end
rescue NoMethodError => ex
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that we had no test coverage for this behavior. In either case we are raising an exception and we can let the caller decider if they want to log and convert the error type themselves 👍

@mmmries
Copy link
Member

mmmries commented Feb 6, 2016

If all the travis checks pass I say :shipit:

abrandoned added a commit that referenced this pull request Feb 6, 2016
…ed_on_load

already loading field on define and should not need to calcualte the …
@abrandoned abrandoned merged commit aa2b15e into master Feb 6, 2016
@abrandoned abrandoned deleted the abrandoned/cache_tag_encoded_on_load branch February 6, 2016 22:41
@film42
Copy link
Member

film42 commented Feb 7, 2016

👍 👍

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