Page MenuHomePhabricator

Performance test and tune DiffusionHistoryListView
Closed, WontfixPublic

Description

See D18006 for discussion, this view library potentially makes a lot of DB calls with the markup engine, it should be batched (since I'm probably going to drop in replace the History view in Diffusion with it.

Event Timeline

chad created this task.May 24 2017, 5:09 PM

@epriestley maybe this should just AJAX?

I think using a Pool (like HandlePool) is not too much more work (at least, if I'm implementing it) than AJAX it and will be useful in more cases. I believe we need the Pool implementation regardless, sooner or later.

If you're comfortable doing an AJAX implementation and want to hook that up, I think that's fine to ship and then I can replace it with a Pool implementation when I have a chance. If this is ending up on my plate either way, I don't think there's a big time savings from doing AJAX.

(There's also maybe some interaction with T9713 here, although we could retrofit that easily enough later.)

chad added a comment.May 26 2017, 4:39 PM

Yeah I just didn't want to add the (...) view to DiffusionHistory if it's going to slow those pages down. I'd just leave the feature out.

chad moved this task from Backlog to UI Refresh on the Diffusion board.Jun 7 2017, 2:55 AM
epriestley closed this task as Wontfix.Apr 15 2019, 3:30 PM
epriestley claimed this task.

There's no specific performance/scale issue here, so waiting until one arises or this pops up on a profile.