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

V1.0.6 taskboard optimization #1095

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ippeiukai
Copy link

This has cut off total about 40-50% of server side response time with a large sprint with number of stories+tasks exceeding 1000.

Changes are:

  • unnecessary loading of status just to get id (task.status.id -> task.status_id) in app/views/rb_taskboards/show.html.erb
  • eliminate loading of task's story when task is actually loaded as a descendant of that story.
  • switch to story_tooltip_ajax instead of rendering rb_stories/tooltip partial
  • avoid repeated render :partial => 'name' which resolves the partial from name every time.

@ctlajoie
Copy link
Contributor

Very interested in these changes since we have performance problems with backlogs, but this doesn't merge cleanly with the current redmine_backlogs master.

@ippeiukai
Copy link
Author

this doesn't merge cleanly

I should have checked that. I'll update my branch as soon as I find a time.

As to the performance problems, the biggest problem of taskboards' user experience actually turned out to be the sheer size of the entire html. It grew linearly to over 1.5 MB with our case, and it takes browsers about 20 seconds to process DOM and render after the data is downloaded. Reducing server response from 20 seconds to 10 is significant, but it seems a more drastic approach might be needed in the big picture.

@ippeiukai
Copy link
Author

@ctlajoie
What was your problem with merge?
Merging to c171804 seems to run without a problem locally. Github also says "This pull request can be automatically merged by project collaborators" here.

$ git merge v1.0.6-taskboard_optimization
Merge made by the 'recursive' strategy.
 app/helpers/rb_common_helper.rb       |  1 +
 app/helpers/rb_partials_helper.rb     | 39 +++++++++++++++++++++++++++++++++++
 app/models/rb_story.rb                | 11 ++++++++++
 app/views/rb_taskboards/show.html.erb |  7 ++++---
 4 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 app/helpers/rb_partials_helper.rb

@waleedjaffar
Copy link

Taskboard tab calculates and loads users color preference for all the tasks listed in Taskboard.
For instance, assume there are 100 users in project and 50 tasks in Taskboard, Then taskboard tab makes 100 x 50 database hits to populate assignee's color preference.

A simple fix for this would be making an Ajax call that loads user's color setting whenever assigne is changed

@ippeiukai
Copy link
Author

I only took a look at easy-to-change problems that showed up on the profiler. I'm sure different performance problem arises with different setup / number of users / number of issues etc.

FYI, Here is the request_profiler result of taskboards#show on a cloned instance before and after this optimisation looking at the same URL. Please notice the logic that hasn't been touched have about twice the percentage now.

before:
taskboards-show-before

after:
taskboards-show-after

Now, new relic shows DOM processing is much more prominent. Note in evenings we have only projects with significantly smaller number of tasks active.
taskboards-show-newrelic-browser

@ctlajoie
Copy link
Contributor

@ippeiukai My mistake. As you say, there is no problem with the merge. :)
This and the task update perf improvements are fantastic. This project appears to be dead so it may never get merged in, but I'll be using them with our customized version of backlogs. Thanks!

@Jellyfrog
Copy link

@AllThatIsTheCase @relaxdiego @Vanuan is this project dead? In that case, maybe @ippeiukai wants to take care of it in his fork or something?

@ippeiukai
Copy link
Author

Unfortunately @ippeiukai does not want to take care of it...

I was only trying to prolong its life a bit for my team. It was already in use when I joined it. We managed to get it performant enough to get it practical again this time, but don't see why we'd choose to continue with this plugin for future projects.

I wish more skilled devs love this plugin, but I suspect devs do not benefit directly from maintaining it unlike team leaders and managers. It's different from some open source software that become directly relevant to their or their clients' products.

@relaxdiego
Copy link
Contributor

@Jellyfrog @ippeiukai I suspect that all other original developers have moved on to other project management software making their future contributions to this project impractical/unfeasible. That was certainly my case a few years back (I prefer to contribute to projects that I personally use because I believe that has a direct effect on quality). If anyone is still using this plugin and is willing to maintain it, I'd be happy to give you commit rights. Otherwise, check out @finnlabs fork of it for OpenProject at https://github.com/finnlabs/openproject-backlogs

@Vanuan
Copy link
Contributor

Vanuan commented Apr 17, 2015

FinnLabs' fork is definitely more active now (and is backed financially, I suspect).
If somebody wants to maintain this fork (due to compatibility issues), please ask Mark to provide commit rights. I'm sure he would happy to give it.

@futaz
Copy link

futaz commented Oct 18, 2016

@waleedjaffar As I know, Rails uses SQL caching, so if you have 100 users and each of them has 50 task in a sprint, it will generate 100x50 SQL queries, but only 100 will hit the database. In the logs you can see CACHE 0.0ms before those queries which had been served from cache.

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.

7 participants