launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

The proposal to merge lp:~lifeless/launchpad/bug-607935 into lp:launchpad has been updated.

Description changed to:

In this iteration I start doing range requests on BugMessage.index and no longer read in every bug message for every page load.

This required refactoring the stack by which such messages are obtained - we really should ditch BugComment I think and just use BugMessage directly (having it delegate to Message for the message fields). But thats a different project.

Anyhow, there were some properties on the bug task view that required access to all messages to work - e.g. the concept of 'how many bug comments are visible with ?show=all present' requires us to load the text of every bug message because of the algorithm in play. I've taken the view that our users will value speed of page loading over absolute precision and adjusted these to show the total messages in the system, rather than the total that would be shown. This is massively faster.

The largest expected benefit from this branch is less Disk IO - e.g. in bug one, we should skip messages 41->1280 or so. Thats a good 5 seconds of IO when it occurs - and it occurs frequently.

As a drive by I fixed the bug message grouping logic to be stable on bugmessage.index if/when two bug comments have the same datetime - which can happen in tests (there was a fragile test failing 1/3rds of the time for me).

I also fixed the logic for putting 'show all message' fillers in to show them where any hidden message is, though still guarded by the overall check for whether we're bulk-hiding messages. This was needed to avoid going up to two identical queries to get different ranges of comments.

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-607935/+merge/49915
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-607935/+merge/49915
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-607935 into lp:launchpad.