HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Redundant User Queries...

mtschirsmtschirs ✭✭✭

Just to display the name and photo of the user that created a discussion, Vanilla (checked with 2.2 master) performs 4 redundant user queries - and that is even after applying this fix: https://github.com/vanilla/vanilla/pull/2916

The user is queried two times via Gdn::userModel->getID() and two times via Gdn::userModel->getIDs().

The user queries probably don't hit the database since there is Gdn::cache() - still, I see room for improvement here... :sweat:

Tagged:

Comments

  • Did you verify this using the Debugger?

    All important models implement a memory cache:

    Gdn::userModel()->getID(5)
    Gdn::userModel()->getID(5)
    Gdn::userModel()->getIDs([5])
    ...
    

    The above will only execute a single query, even without an active cache.

    The second time the same ID is being requested it comes from a cache array which is a property of the model.

    This works throughout the request lifetime since the model instance is always being reused (you should write Gdn::userModel(); never instantiate a new UserModel if you don't need to).

    Bottom line is that you can freely call UserModel::getID with IDs that have already been fetched.

  • LincLinc Detroit Admin

    @Bleistivt said:
    Bottom line is that you can freely call UserModel::getID with IDs that have already been fetched.

    Precisely. We make a basic assumption that anyone interested in scaling Vanilla is going to use memcached at the very least. It's baked in.

  • mtschirsmtschirs ✭✭✭

    I was rather wondering about all the redundant user-join-ins... The code reads as if the coder of each part forgot about already having all the information at his disposal. Could surely use a refactoring :pleased:

Sign In or Register to comment.