Small, modular, and… slow?

NodeBB is zippy. It’s an underdog, the David to the Goliaths of the forum community.

A large part of why that is is because we don’t implement features that slow down the overall system. We leverage Node.js’ event loop to eliminate unnecessary idling, and we defer as much as we can to the client-side unless absolutely necessary.

Another reason is because we don’t implement every single feature we see willy-nilly. Quality over quantity, or if you prefer: It’s is not what you put in, it’s what you leave behind.*

In any case, one particular issue we’d been noticing on and off is that adding particular features caused an unexpected addition to the overall load time of API calls. Usually, some fine-tuning is in order, but more often than not, it does boil down to this one simple fact:

Small reusable methods are always preferable to larger "god" methods, but this benefit comes at a cost: It’s very easy to forget that these methods still take time.

Put another way, while it is definitely easier from a maintenance standpoint to construct many small methods to achieve your goal, do not fall into the trap of assuming that small methods are fast methods.

Example

We recently** added user groups to NodeBB, and a community pull request extended the system to allow groups to contain other groups (*insert xzibit meme*) as well. We implemented usage of groups across the board*** to enable organization of moderators, administrators, and category access permissions. At this point, we noticed a slowdown in our API calls due to method call abstractions at scale: working with groups was a three step process:

1. Find a group’s gid given its name
2. Check if it was deleted or empty
3. Carry out its operation (e.g. check if a uid was a member or not)

What I soon realised was that steps 1 and 2 were completely pointless, and even though they were both very fast methods, collectively, we were running them hundreds of times!

It really goes to show how small methods could easily fly under the radar and end up slowing down a nimble application!

I realise now that I made several incorrect assumptions:

1. There was no point in preserving deleted groups.
2. Both the gid and groupName were unique keys, and I fell into the trap of always thinking that I needed a *id-style primary key

Nobody really cared if a group was deleted or not, and having to handle specific edge cases related to "deleted but not actually deleted" groups was wasting precious cycles. Precious cycles I could normally spare, but not when they were amplified 100 times!

The solution was straightforward — I migrated all groups and removed the gid altogether. I then changed the internal methods so that if a group was deleted, it was gone for good, and re-created if necessary. Deleting and re-creating groups are small methods too, and run much less frequently (or not at all) compared to an isDeleted or isEmpty method call.

All in all, we reduced the load time of the /api/home route from ~965ms (good god!!) to ~170ms. That was how significant those two little methods were in the end.

* Pretty sure I butchered the quote.

** I play fast and loose with adverbs when it comes to time… "recently" in this case means 4-5 months

*** Get it? Across the "board"?