Code review comment for ~pappacena/turnip:paginated-check-refs-permissions

Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Hi, cjwatson.

- I'll check both problems you've mentioned on netstring, and open separated MPs for that. I didn't spot those problems when checking, and thanks for pointing that out.

- GitRepository.checkRefPermissions would be my next step, to see query count and other common performance problems. I'll check that too.

- I was planning to adjust the page size after benchmarking a bit better. Yesterday night I've run some quick tests locally, and already realized that 25 was way too small (before your comment, I had already pushed a change to make it 100). The problem is that the reported bug was trying to push 2.000 branches.

- The pagination and parallelization was actually not that complicated. Apart from the changes on where we do the socket creation, make it run in parallel was a change of about 15 lines of code, for example. But I agree that most of our problems are probably somewhere else.

« Back to merge proposal