So I had a great performance tuesday; two patches up, and only a few hours fighting storm :).
I started looking at bug 607935 - bugtask:+index timing out. e.g. https://bugs.launchpad.net/ubuntu/+bug/1 Its possibly not the cause, but *at least* 1.2 seconds of the page is spent doing over 300 queries for the logic needed to fill out the 'unsubscribe team x' actions. I suspect Bug.indexed_messages is also a problem - I filed a bug on this yesterday. In doing this patch I extended StormStatementRecorder for use with HasQueryCount, as there are view tests that were close to the code, so I wanted to get the nice failure output of HasQueryCount without driving the whole template - in case other parts of the template scale badly this let me focus in. To address it, I just changed a python expression to a SQL one: for team in self.user.teams_participated_in: if bug.isSubscribed(team) or bug.isSubscribedToDupes(team): ... Those two lines were the cause of (for me) 330 queries :) In doing it I added a new helper along the line of existing ones to IBug, it would have been nice to use GenericCollections or something, but the need wasn't quite as high as the effort. Michael Hudson kindly reviewed this midday for me. I then reviewed Tim's Cloud optimisation patch, which should nuke the timeouts on the code cloud \o/. I then started poking at bug 618367 : Distribution:+assignments timeouts, which is up near the top of the candidate-timeouts report. This is e.g. https://launchpad.net/ubuntu/+assignments. A major problem, according to an oops, is ValidPersonCache : 6 seconds to do 170 lookups there. This was pretty shallow, except for me making a beginners mistake in storm. - refactor the preloading code for Person._all_members to let the valid person checks be reused. - add a test that the page query count doesn't scale up with as more unique assignees etc are ysed The storm issues I had today: Class.some_reference == Class.some_reference blows up in an inexplicible way. It would be awesome for this to work better. I filed a bug. LeftJoin(aliased_table, otheraliased_table.id==aliased_table.reference) compiles to LeftJoin .. on otheraliased_table.id = OriginalTable.foreignkey (this is a variation on the first issue). What is needed is: LeftJoin(aliased_table, otheraliased_table.id==aliased_table.referenceID) (for SQLBase) or LeftJoin(aliased_table, otheraliased_table.id==aliased_table.reference_id) (for native storm) There was another stormish issue I ran into. In precaching this stuff, it was an extension of the pattern I experimented with on Person:+participants In this case, we had three separate Persons to preload per Specification. Now, there are two ways you can do this in SQL (without views - we're removing a terrible-performance view): - add the three (aliased) tables once per person relationship. So we have 10 tables being returned in the query. - join with an IN clause, where any of the assignee/drafter/approvers can join to Person, and use the EmailAddress and Account tables only once. The latter way is a lot easier to read, and probably simpler for the query planner to tune, but our mapping logic really doesn't fit well with it: - [0] is poorly defined (you need N rows - up to 3, per index in the result). - [N:M] is even more poorly defined. - Iteration needs to be cleverer - you need to buffer the Specification until all possible rows are seen, then yield it. I filed a bug on storm saying it would be nice to have built in support to handle this more gracefully - storm *will* need it as it grows eager loading facilities. A big thanks to James Henstridge and Stuart for giving me a hand when I was getting ready to headbutt storm for miscompiling things. In other news, the top OOPSer for the last month or so is now fixed on edge - e.g. https://api.edge.launchpad.net/1.0/bugs/414746/attachments - I don't think I'm going to worry about a CP for it, Leanne said the QA scripts handle LP dying :( - but next rollout should see this drop right off the radar - its down to about one second for me. Now, I'm off to figure out why I got two seemingly unrelated test failures from my first patch. -Rob _______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
