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

Ruby C extension speedup: don't re-intern constant string needlessly. #405

Merged
merged 1 commit into from May 19, 2015

Conversation

cfallin
Copy link
Contributor

@cfallin cfallin commented May 19, 2015

Previously, the C extension interned the string "descriptor" (to obtain a Ruby ID value, which is an integer representation of an interned string) every time it needed to look up a descriptor object given a message class. This caused the interned-string table lookup, which uses a hashtable, to become a performance hotspot. Since the string is a constant, it makes much more sense to intern it once at initialization then use that value.

Also fixed lines with > 80 char length.

@@ -35,6 +35,11 @@
// -----------------------------------------------------------------------------

const char* kDescriptorInstanceVar = "descriptor";
ID descriptor_instancevar_interned;
Copy link
Member

Choose a reason for hiding this comment

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

Since this is used in many files, seems to make sense to have it in the main file? Then you could just initialize it directly without needing a separate function.

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.

@cfallin
Copy link
Contributor Author

cfallin commented May 19, 2015

Thanks! PTAL.

@haberman
Copy link
Member

LGTM

haberman added a commit that referenced this pull request May 19, 2015
Ruby C extension speedup: don't re-intern constant string needlessly.
@haberman haberman merged commit 4324bf6 into protocolbuffers:master May 19, 2015
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
Updated Bazel fuzzing to use the newest fuzz rules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants