Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optimize ThreadTools.get_latest_undeleted_pid #320

Closed
julianlam opened this issue Sep 22, 2013 · 3 comments
Closed

Optimize ThreadTools.get_latest_undeleted_pid #320

julianlam opened this issue Sep 22, 2013 · 3 comments
Assignees
Milestone

Comments

@julianlam
Copy link
Member

Load testing by @adarqui and further examination of the codebase has revealed that simply entering a category and rendering the topic listing causes NodeBB to iterate into each and every topic and parse its posts _completely_.

In other words, simply by going into a category, NodeBB is doing the equivalent work as if you had gone into every single topic at the same time.

Code tracing has revealed the following pattern:

load category -> get topics -> for each topic, get a teaser

getTeaser calls get_latest_undeleted_pid (which, by the way, should be camelCased), which calls getPostsByTid, which loads and renders every single post in order to find which post has been deleted. Yikes.

This should cut down the load time significantly.

@ghost ghost assigned julianlam Sep 22, 2013
@psychobunny
Copy link
Contributor

At some point in the past, the teaser text was stored along with topic data. Not sure why that got changed. In any case the new design removes the teaser text altogether, so nothing to worry about for now!

@julianlam
Copy link
Member Author

Agreed, the new design removes the teaser, but get_latest_undeleted_pid is still required in order to find out who the last poster was.

The method is also used for thread auto-deletion/restoration, so it still needs to be rewritten.

@psychobunny
Copy link
Contributor

Yeah I'd just leave it how it was in the first place I imagine it was "optimized" to prevent duplicated info ;p

julianlam added a commit that referenced this issue Sep 22, 2013
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants