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

Slight optimisation on cache hit without match. #3

Closed
wants to merge 1 commit into from
Closed

Slight optimisation on cache hit without match. #3

wants to merge 1 commit into from

Conversation

Aracthor
Copy link

Actually, on cache hit, there is a new pointless dynamic_cast done if ptr is not an instance of required class.

It would be slightly faster to return directly nullptr if there is a cache hit but if this_vtable doesn't match with the stored one.

@tobspr
Copy link
Owner

tobspr commented Dec 17, 2016

I believe this does not work, imagine the following:

// Class hierarchy is A -> B -> C
A* a = new B;
fast_dynamic_cast<B*>(a);
// Cached offset is now A->B, Cached vtable is [A::vftable]

A* a2 = new C;
fast_dynamic_cast<B*>(a2);
// With your change, this returns zero, because
// if(src_vtable_ptr == this_vtable) fails and thus a nullptr is returned

I will let it run through my unit tests on monday though, to give you a definite answer.

@Aracthor
Copy link
Author

You are right, I didn't consider that case. Forget about it.

@Aracthor Aracthor closed this Dec 17, 2016
@Aracthor Aracthor deleted the slight_optimization branch December 17, 2016 12:14
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.

2 participants