Re: [edk2] [PATCH] ShellPkg : Cache the environment variable into memory to enhance the performance.

2016-04-11 Thread Carsey, Jaben 
2 comments below.

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Qiu, Shumin
> Sent: Sunday, April 10, 2016 5:55 AM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; Carsey, Jaben
> ; Ni, Ruiyu 
> Subject: [PATCH] ShellPkg : Cache the environment variable into memory to
> enhance the performance.
> Importance: High
> 
> Currently UEFI Shell reads variable storage to get the environment
> variables every time running a new command. And reading(writing)
> UEFI variables is a high cost operation on most platforms. In order
> to enhance the performance this patch read the variable storage once
> and cache the environment variables in memory. Every further 'set'
> command will save the variable not only to Shell cache, but also the
> flash variable storage.
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin 
> ---
>  ShellPkg/Application/Shell/Shell.c |   4 +
>  ShellPkg/Application/Shell/ShellEnvVar.c   | 175
> -
>  ShellPkg/Application/Shell/ShellEnvVar.h   |  82 +-
>  ShellPkg/Application/Shell/ShellProtocol.c | 101 ++---
>  4 files changed, 317 insertions(+), 45 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index bd695a4..12daff9 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -445,6 +445,8 @@ UefiMain (
>  Status = CommandInit();
>  ASSERT_EFI_ERROR(Status);
> 
> +Status = ShellInitEnvVarList ();
> +
>  //
>  // Check the command line
>  //
> @@ -702,6 +704,8 @@ FreeResources:
>  DEBUG_CODE(ShellInfoObject.ConsoleInfo = NULL;);
>}
> 
> +  ShellFreeEnvVarList ();
> +
>if (ShellCommandGetExit()) {
>  return ((EFI_STATUS)ShellCommandGetExitCode());
>}
> diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c
> b/ShellPkg/Application/Shell/ShellEnvVar.c
> index a8f177e..d2c721d 100644
> --- a/ShellPkg/Application/Shell/ShellEnvVar.c
> +++ b/ShellPkg/Application/Shell/ShellEnvVar.c
> @@ -1,7 +1,7 @@
>  /** @file
>function declarations for shell environment functions.
> 
> -  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -17,6 +17,11 @@
>  #define INIT_NAME_BUFFER_SIZE  128
>  #define INIT_DATA_BUFFER_SIZE  1024
> 
> +//
> +// The list is used to cache the environment variables.
> +//
> +ENV_VAR_LIST   gShellEnvVarList;
> +
>  /**
>Reports whether an environment variable is Volatile or Non-Volatile.
> 
> @@ -379,3 +384,171 @@ SetEnvironmentVariables(
>//
>return (SetEnvironmentVariableList(>Link));
>  }
> +
> +/**
> +  Find an environment variable in the gShellEnvVarList.
> +
> +  @param KeyThe name of the environment variable.
> +  @param Value  The value of the environment variable, the buffer
> +shoule be freed by the caller.
> +  @param ValueSize  The size in bytes of the environment variable
> +including the tailing CHAR_NELL.
> +  @param Atts   The attributes of the variable.
> +
> +  @retval EFI_SUCCESS   The command executed successfully.
> +  @retval EFI_NOT_FOUND The environment variable is not found in
> +gShellEnvVarList.
> +
> +**/
> +EFI_STATUS
> +ShellFindEnvVarInList (
> +  IN  CONST CHAR16*Key,
> +  OUT CHAR16  **Value,
> +  OUT UINTN   *ValueSize,
> +  OUT UINT32  *Atts OPTIONAL
> +  )
> +{
> +  ENV_VAR_LIST  *Node;
> +
> +  if (Key == NULL || Value == NULL || ValueSize == NULL) {
> +return SHELL_INVALID_PARAMETER;
> +  }
> +
> +  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
> +  ; !IsNull(, >Link)
> +  ; Node = (ENV_VAR_LIST*)GetNextNode(, 
> >Link)
> + ){
> +if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {
> +  *Value  = AllocateCopyPool(StrSize(Node->Val), Node->Val);
> +  *ValueSize  = StrSize(Node->Val);
> +  if (Atts != NULL) {
> +*Atts = Node->Atts;
> +  }
> +  return EFI_SUCCESS;
> +}
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> +/**
> +  Add an environment variable into gShellEnvVarList.
> +
> +  @param KeyThe name of the environment variable.
> +  @param Value  The value of environment variable.
> +  @param ValueSize  The size in bytes of the environment variable
> +including the tailing CHAR_NULL
> +  @param Atts   The attributes of the variable.
> +
> +**/

[edk2] [PATCH] ShellPkg : Cache the environment variable into memory to enhance the performance.

2016-04-10 Thread Qiu Shumin 
Currently UEFI Shell reads variable storage to get the environment
variables every time running a new command. And reading(writing)
UEFI variables is a high cost operation on most platforms. In order
to enhance the performance this patch read the variable storage once
and cache the environment variables in memory. Every further 'set'
command will save the variable not only to Shell cache, but also the
flash variable storage.

Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 ShellPkg/Application/Shell/Shell.c |   4 +
 ShellPkg/Application/Shell/ShellEnvVar.c   | 175 -
 ShellPkg/Application/Shell/ShellEnvVar.h   |  82 +-
 ShellPkg/Application/Shell/ShellProtocol.c | 101 ++---
 4 files changed, 317 insertions(+), 45 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index bd695a4..12daff9 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -445,6 +445,8 @@ UefiMain (
 Status = CommandInit();
 ASSERT_EFI_ERROR(Status);
 
+Status = ShellInitEnvVarList ();
+
 //
 // Check the command line
 //
@@ -702,6 +704,8 @@ FreeResources:
 DEBUG_CODE(ShellInfoObject.ConsoleInfo = NULL;);
   }
 
+  ShellFreeEnvVarList ();
+
   if (ShellCommandGetExit()) {
 return ((EFI_STATUS)ShellCommandGetExitCode());
   }
diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c 
b/ShellPkg/Application/Shell/ShellEnvVar.c
index a8f177e..d2c721d 100644
--- a/ShellPkg/Application/Shell/ShellEnvVar.c
+++ b/ShellPkg/Application/Shell/ShellEnvVar.c
@@ -1,7 +1,7 @@
 /** @file
   function declarations for shell environment functions.
 
-  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -17,6 +17,11 @@
 #define INIT_NAME_BUFFER_SIZE  128
 #define INIT_DATA_BUFFER_SIZE  1024
 
+//
+// The list is used to cache the environment variables.
+//
+ENV_VAR_LIST   gShellEnvVarList;
+
 /**
   Reports whether an environment variable is Volatile or Non-Volatile.
 
@@ -379,3 +384,171 @@ SetEnvironmentVariables(
   //
   return (SetEnvironmentVariableList(>Link));
 }
+
+/**
+  Find an environment variable in the gShellEnvVarList.
+
+  @param KeyThe name of the environment variable.
+  @param Value  The value of the environment variable, the buffer
+shoule be freed by the caller.
+  @param ValueSize  The size in bytes of the environment variable
+including the tailing CHAR_NELL.
+  @param Atts   The attributes of the variable.
+
+  @retval EFI_SUCCESS   The command executed successfully.
+  @retval EFI_NOT_FOUND The environment variable is not found in
+gShellEnvVarList.
+
+**/
+EFI_STATUS
+ShellFindEnvVarInList (
+  IN  CONST CHAR16*Key,
+  OUT CHAR16  **Value,
+  OUT UINTN   *ValueSize,
+  OUT UINT32  *Atts OPTIONAL
+  )
+{
+  ENV_VAR_LIST  *Node;
+  
+  if (Key == NULL || Value == NULL || ValueSize == NULL) {
+return SHELL_INVALID_PARAMETER;
+  }
+
+  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
+  ; !IsNull(, >Link)
+  ; Node = (ENV_VAR_LIST*)GetNextNode(, >Link)
+ ){
+if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {
+  *Value  = AllocateCopyPool(StrSize(Node->Val), Node->Val);
+  *ValueSize  = StrSize(Node->Val);
+  if (Atts != NULL) {
+*Atts = Node->Atts;
+  }
+  return EFI_SUCCESS;
+}
+  }
+
+  return EFI_NOT_FOUND;
+}
+
+/**
+  Add an environment variable into gShellEnvVarList.
+
+  @param KeyThe name of the environment variable.
+  @param Value  The value of environment variable.
+  @param ValueSize  The size in bytes of the environment variable
+including the tailing CHAR_NULL
+  @param Atts   The attributes of the variable.
+
+**/
+VOID
+ShellAddEnvVarToList (
+  IN CONST CHAR16 *Key,
+  IN CONST CHAR16 *Value,
+  IN UINTNValueSize,
+  IN UINT32   Atts
+  )
+{
+  ENV_VAR_LIST  *Node;
+  
+  if (Key == NULL || Value == NULL || ValueSize == 0) {
+return;
+  }
+
+  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
+  ; !IsNull(, >Link)
+  ; Node = (ENV_VAR_LIST*)GetNextNode(, >Link)
+ ){
+if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {
+  Node->Atts = Atts;
+  SHELL_FREE_NON_NULL(Node->Val);
+  Node->Val  = AllocateZeroPool (ValueSize);
+  ASSERT (Node->Val != NULL);
+  CopyMem(Node->Val, Value, ValueSize);
+  return;
+}
+  }