440466 - Remove getAuthors call from getListAddons

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Wil Clouser [:clouserw]]

For performance reasons (and because it's superfluous?) we need to remove this call.  Instead of doing this call separately it should be a part of the HABTM relationship between authors and add-ons.

Comment 1

[Fred Wenzel [:wenzel]]

Attachment: removing getAuthors call, using HABTM association rule

there you go.

Comment 2

[Michael Morgan [:morgamic]]

Part of the reason fligtar did it this way was that cake doesn't work well with metadata stored in a map table.  In this case addons_users stores the meta for listed/position... so it's not pointless, we should just find a better way I think.

Comment 3

[Wil Clouser [:clouserw]]

Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

adding morgamic to the review list since he is set up to make sure this patch performs well

Comment 4

[Wil Clouser [:clouserw]]

Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Functionally, it works for me.  My r+ dependent on morgamic's perf testing.

Comment 5

[Michael Morgan [:morgamic]]

Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Could you add the appropriate indexes to addons_users as well?

Comment 6

[Fred Wenzel [:wenzel]]

(In reply to comment #4)
> (From update of attachment 325788 [details] [diff] [review])
> My r+ dependent on morgamic's perf testing.

Okay let's wait for that. Though I vote for checking this in like this anyway even if it doesn't "dramatically" improve system load, as it's cleaner than what we have now. (Unless of course my patch has a negative impact, which I doubt).

Comment 7

[Michael Morgan [:morgamic]]

I saw 12k -> 5k for dbsize, so that looks like it has the desired effect.

Comment 8

[Fred Wenzel [:wenzel]]

(In reply to comment #5)
> (From update of attachment 325788 [details] [diff] [review])
> Could you add the appropriate indexes to addons_users as well?

We already have an index over addon_id and user_id, and adding additional indexes won't make them being used by MySQL.

Comment 9

[Justin Scott [:fligtar]]

I tested and everything works fine for me as well - Thanks.

Comment 10

[Michael Morgan [:morgamic]]

Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Nevermind, index won't help, +r=me

Comment 11

[Fred Wenzel [:wenzel]]

Thanks, everyone. r16058.