On 2/19/24 12:37, Gerd Hoffmann wrote: > Hi, > >> (I'm missing the larger picture here -- is this related to the problem >> -- too many CPUs to fit their infos into a single HOB -- that Pawel >> worked on for a while? > > Different HOB, but similar underlying problem. > > At least the HOB structure already has the fields needed to allow > splitting the information into multiple HOBs, even though the current > code doesn't actually do that (which is fixed by this series). > >> The outer loop is suboptimal, IMO, to just open-coding another HOB scan >> -- this approach looks quadratic, even though it could be linear. More >> or less, as proposed, we call GetMpHandOffHob() for each MP_HAND_OFF >> HOB, which will scan n/2 HOBs on average. (Even if the GUID HOB list is >> sorted by ProcessorIndex, we'll scan 1 + 2 + 3 +... HOBs.) But if we >> open-coded GetFirstGuidHob() and GetNextGuidHob() here, then a single >> scan would suffice. > > Ray raised performance concerns too. > > Does HobLib provides any ordering guarantees? Specifically in case the > HOBs are returned in the same order they are created (which implies they > are sorted by ProcessorIndex because patch #5 creates them in that > order) this can certainly be optimized.
I don't think there are ordering guarantees, but my larger point in review is that the order does not matter. I see no reason for addressing HOBs in increasing processor index order. No loop over the HOBs that I've seen touched by this patch set seems to depend on the visiting order of HOBs. So the outer loops that advance with GetMpHandOffHob(ProcessorIndex) should be rewritable with get-next-GUID-HOB. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115596): https://edk2.groups.io/g/devel/message/115596 Mute This Topic: https://groups.io/mt/104369845/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
