On 1/12/24 03:44, Andrew (EFI) Fish wrote: > Sorry need some more time to digest this…. First thoughts. > > 1) The actual performance issue we hit was the explosion > of CoreValidateHandle() calls as the number of protocols got large for > some diags. The newer handles tended to be at the end of the list if I > remember correctly. > a) It looks like CoreValidateHandle() is the only > place *gHandleList* was walked, as the handle info is crossed > referenced in the protocol database. So that is why we changed that. > 2) If the issue at hand is MM why not just drop the optimizations?
Yes, that's the first (and easy) idea. But I guess it needs some measurements (no perf regression). It would be nice to know whether Intel (?) measured any serious perf gains when they originally implemented (in the 2000s?) the optimization. > b) If we have so many MM protocols and handles that seems like its own > problem? I would agree; however, IIRC, the depex evaluator for the MM drivers also searches the UEFI (DXE) protocol database, not just the MM protocol database. (Independently: I think that's a valid thing to do for *SMM* drivers, because the entry point functions of those drivers are permitted to use both SMM and DXE/UEFI protocols. But whether the same is valid for the *standalone* MM drivers -- that looks questionable. Standalone MM drivers should not depend on UEFI/DXE protocols ever, IIUC.) > 3) The issue is patching the grammar in place, why can’t we just make a > copy for the dispatcher grammer, and operate on the copy. Maybe via a > copy on 1st update strategy? Yes, copying the depex to the heap, and patching it there, was Nhi's #1 fix proposal. I think that could be made work. But I'm not sure if the perf savings are worth the additional complexity. The heap allocation (where the writeable depex would exist) would have to be permanently associated with the loaded PE image -- because the dispatcher might need to reevaluate the depex across multiple rounds of dispatching. So that's a new field in some image-related structure, it also needs to be freed upon unload (?), what if the memory allocation fails during depex eval (just consider the depex to eval to FALSE?), etc. Doable, but hairy; not sure if the perf is worth that effort. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113704): https://edk2.groups.io/g/devel/message/113704 Mute This Topic: https://groups.io/mt/103594587/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
