Re: Slow(er) loading of Grub starting after commit 887f98f0d

[Daniel Kiper]

On Thu, Sep 08, 2022 at 03:49:03PM +0800, Gary Lin wrote:
> On Wed, Sep 07, 2022 at 06:21:07PM +0200, Daniel Kiper wrote:
> > Adding folks who may be interested in this...
> >
> > Sorry for delay but I was on vacation...
> >
> > Somehow related issue was reported here [1]...
> >
> > On Fri, Sep 02, 2022 at 01:45:28PM +0800, Gary Lin via Grub-devel wrote:
> > > On Thu, Sep 01, 2022 at 06:44:24PM +0200, Marcel Langner via Grub-devel 
> > > wrote:
> > > > Hi,
> > > > just subscribed coming from arch forum
> > > > (https://bbs.archlinux.org/viewtopic.php?id=279006) to report slower 
> > > > loading
> > > > of grub after commit 887f98f0d.
> > > > The additional delay is around 20s and happens right after I get the 
> > > > message
> > > > Slot 0 opened (as I have a luks encrypted partition) and after 
> > > > selecting a
> > > > menu entry.
> > > I also encountered a similar delay when backporting
> > > 887f98f0db4..1df2934822df to openSUSE grub2. The loading of the grub2
> > > menu with openSUSE theme was delayed around 2~3 seconds after applying
> > > the patch set. Without the patch set, the menu showed right after the
> > > "Welcome to Grub" message.
> > >
> > > > From what I have figured out already a new way of allocating memory more
> > > > dynamically was introduced.
> > > > So I played a bit with the code and figured out, that increasing the 
> > > > newly
> > > > introduced DEFAULT_HEAP_SIZE speeds up the boot process again, until it 
> > > > has
> > > > reached no noticeable change from before. At 2MB it was already faster
> > > > loading, at 4MB I could not see any difference from before the change
> > > > anymore.
> > > >
> > > For my case, I have to increase the default heap size to 8MB to eliminate
> > > the delay totally.
> > >
> > > > My guess is that the now additional calls to get the needed memory slows
> > > > down the process. Not all users that reported in the arch forum seem to 
> > > > be
> > > > impacted. I guess it depends on what modules they use and how much 
> > > > memory
> > > > internally is used depending on what is individually configured and how
> > > > (e.g. encryption, type of root fs...).
> > > >
> > > > I understood the former logic of allocation in a way that (if needed) 
> > > > either
> > > > a maximum 1.6GB of memory is allocated or 1/4 of the installed memory. 
> > > > For
> > > > my system the new code now starts at 1MB (instead of over 1GB) and 
> > > > needs to
> > > > work its way up.
> > > > This new way of allocating seems to try to not simply cancel if a hard 
> > > > coded
> > > > allocation value fails but starts at a very low minimum and tries until 
> > > > the
> > > > available system memory is really exhausted. So basically tries to also
> > > > support lower memory situations better but assuming a minimum of 1MB.
> > > > So far my understanding.
> > > >
> > > > Assuming my findings can be supported by you, maybe an adjusted 
> > > > allocation
> > > > scheme where 1/4 of the available memory can be used as the default and 
> > > > if
> > > > this is not 1MB one could still cancel and throw errors?
> > > >
> > > It sounds feasible to bring back the old heap size setting since the 
> > > setting
> > > already works for years, and the extra memory demands can be covered by 
> > > the
> > > new dynamic allocation patches.
> >
> > IMO it is an overkill. I think we should increase DEFAULT_HEAP_SIZE to
> > 32 MiB first as it was suggested by Daniel A. in the [1] thread. Or more
> > if needed. I would revert old behavior if increasing DEFAULT_HEAP_SIZE
> > does not work well.
> >
> 32 MiB sounds a good start. Besides the default heap size, I'm thinking
> about setting a minimal size, say 1 MiB, for dynamic allocation. For
> example, when the default heap is used up and the verifier is loading
> several small files, allocating a large enough chunk beforehand may
> reduce the frequency to go through the memory allocation path.

Make sense for me.

I did a bit deeper investigation and found out that latest Debian
testing kernel size is 7.3 MiB and the initrd with all drivers size is
43 MiB. If we assume we will use 32 MiB as DEFAULT_HEAP_SIZE then the
GRUB will need to call EFI alloc functions at least once. Potentially we
could increase initial size to, e.g., 64 MiB. However, I think we should
stay with 32 MiB to reduce minimal memory requirements to bare minimum
which does not impact performance.

Daniel