Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore

["Dandan Bi" <dandan.bi@intel.com>]

Hi Liming,

Yes, with this change, the performance is better than before. 
Especially for the big formset with lots of configuration, it has better use experience when loading the formset to display. 

Could you help review this patch? Thanks.


Thanks,
Dandan
-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Wednesday, August 2, 2023 1:08 PM
To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
Subject: 回复: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore

Dandan:
  Have you collected the performance data for this enhancement? Is the updated one better than before?

Thanks
Liming
> -----邮件原件-----
> 发件人: Dong, Eric <eric.dong@intel.com>
> 发送时间: 2023年7月31日 13:04
> 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via 
> GetVariable for EfiVarStore
> 
> Reviewed-by: Eric Dong <eric.dong@intel.com>
> 
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Monday, July 31, 2023 8:46 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric 
> <eric.dong@intel.com>
> Subject: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via 
> GetVariable for EfiVarStore
> 
> For EfiVarStore (EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER), it will call 
> ExtractConfig-GetVariable-HiiBlockToConfig-ConfigToBlock when load 
> storage value in LoadStorage function. It's not necessary and costs 
> lots of time
to do
> the conversion between config and block.
> So now enhance it to call GetVariable directly.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
> v2: Fix coding style issue.
> 
>  .../Universal/SetupBrowserDxe/Setup.c         | 54 +++++++++++--------
>  1 file changed, 32 insertions(+), 22 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> index 5158baf5bd..2f7b11b1aa 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> @@ -5634,32 +5634,42 @@ LoadStorage (
>      ConfigRequest = Storage->ConfigRequest;
>    }
> 
> -  //
> -  // Request current settings from Configuration Driver
> -  //
> -  Status = mHiiConfigRouting->ExtractConfig (
> -                                mHiiConfigRouting,
> -                                ConfigRequest,
> -                                &Progress,
> -                                &Result
> -                                );
> -
> -  //
> -  // If get value fail, extract default from IFR binary
> -  //
> -  if (EFI_ERROR (Status)) {
> -    ExtractDefault (FormSet, NULL, EFI_HII_DEFAULT_CLASS_STANDARD,
> FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, 
> TRUE);
> -  } else {
> +  if (Storage->BrowserStorage->Type ==
> + EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER) {
>      //
> -    // Convert Result from <ConfigAltResp> to <ConfigResp>
> +    // Call GetVariable directly for EfiVarStore
>      //
> -    StrPtr = StrStr (Result, L"&GUID=");
> -    if (StrPtr != NULL) {
> -      *StrPtr = L'\0';
> +    Status = gRT->GetVariable (Storage->BrowserStorage->Name,
> &(Storage->BrowserStorage->Guid), NULL, (UINTN 
> *)(&(Storage->BrowserStorage->Size)),
> Storage->BrowserStorage->EditBuffer);
> +    if (EFI_ERROR (Status)) {
> +      ExtractDefault (FormSet, NULL,
> EFI_HII_DEFAULT_CLASS_STANDARD,
> + FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, 
> + TRUE);
>      }
> +  } else {
> +    //
> +    // Request current settings from Configuration Driver
> +    //
> +    Status = mHiiConfigRouting->ExtractConfig (
> +                                  mHiiConfigRouting,
> +                                  ConfigRequest,
> +                                  &Progress,
> +                                  &Result
> +                                  );
> 
> -    Status = ConfigRespToStorage (Storage->BrowserStorage, Result);
> -    FreePool (Result);
> +    //
> +    // If get value fail, extract default from IFR binary
> +    //
> +    if (EFI_ERROR (Status)) {
> +      ExtractDefault (FormSet, NULL, EFI_HII_DEFAULT_CLASS_STANDARD,
> FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, 
> TRUE);
> +    } else {
> +      //
> +      // Convert Result from <ConfigAltResp> to <ConfigResp>
> +      //
> +      StrPtr = StrStr (Result, L"&GUID=");
> +      if (StrPtr != NULL) {
> +        *StrPtr = L'\0';
> +      }
> +
> +      Status = ConfigRespToStorage (Storage->BrowserStorage, Result);
> +      FreePool (Result);
> +    }
>    }
> 
>    Storage->BrowserStorage->ConfigRequest = AllocateCopyPool (StrSize 
> (Storage->ConfigRequest), Storage->ConfigRequest);
> --
> 2.39.1.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107512): https://edk2.groups.io/g/devel/message/107512
Mute This Topic: https://groups.io/mt/100520724/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-