Code review comment for ~raharper/curtin:feature/grub-resilient-boot

On Thu, Apr 16, 2020 at 09:20:56PM -0000, Ryan Harper wrote:
> > + # insert the primary esp as first element
> > + storage_grub_devices = ([primary_esp] + [dev for dev in
>
> +1
>
> timeit shows for a small list; it's faster to remove than to iterate the whole list.

Yep, I would anticipate this is for a couple of reasons: firstly, remove
doesn't (necessarily) iterate the whole list, it is complete once it
finds the first matching item. And, secondly, it's written in C and
your list comprehension isn't. ;)

> (crispyboi) curtin % cat bench.py
> #!/usr/bin/python3
> import timeit
> benchmark1 = '''
> primary_esp = '/dev/vda1'
> efi_boot_parts = ['/dev/vda1', '/dev/vdb1', '/dev/vdc1']
> grub_devs = [primary_esp] + [dev for dev in efi_boot_parts if dev != primary_esp]
> '''
> benchmark2 = '''
> primary_esp = '/dev/vda1'
> efi_boot_parts = ['/dev/vda1', '/dev/vdb1', '/dev/vdc1']
> efi_boot_parts.remove(primary_esp)
> grub_devs = [primary_esp] + efi_boot_parts
> '''
> print('Bench1')
> print(timeit.timeit(stmt=benchmark1))
> print('Bench2')
> print(timeit.timeit(stmt=benchmark2))
> (crispyboi) curtin % python3 bench.py
> Bench1
> 0.3933144999900833
> Bench2
> 0.16941187201882713