Code review comment for lp://qastaging/~dpb/tarmac/set-milestone-2

Revision history for this message
Данило Шеган (danilo) wrote :

Hi David,

Thanks for bearing with me. Most everything looks good, but you named your new test file

  test_tarmac_pluign.py

The rest is really good now, thanks.

  review approve

> OK, I took your suggestion. Though I still like the previous one as I
> found it easier to follow. I was curious, so I timed some stuff:
>
> 1000 milestones, 10,000 iterstions:
>
> old: 46.1424999237
> new: 44.1123301983
>
> 1,000,000 milestones, 10 iterations:
>
> old: 57.2593920231
> new: 49.6416790485
>
> As expected, n log n nature of the sort is slowing things down when large
> amounts of records come into play. And... I think this is quickly
> entering a theoretical discussion. :)

Heh, indeed. Thanks for testing it out: the difference might rather amount to
Python memory management (you are still dealing with duplicated structures in
the original variant) since the loop is much more complicated (and thus
slower) in "my" proposal (and sorted() is heavily optimized in Python).

So, in the end, it was probably not worth it even for large sets of
milestones, but it was fun nevertheless :)

review: Approve

« Back to merge proposal