[edk2-devel] [PATCH 1/8] ShellPkg/Comp: add file buffering
Laszlo Ersek
lersek at redhat.com
Mon Jan 4 15:42:28 UTC 2021
The COMP shell command compares two files byte for byte. In order to
retrieve the bytes to compare, it currently invokes
gEfiShellProtocol->ReadFile() on both files, using a single-byte buffer
every time. This is very inefficient; the underlying
EFI_FILE_PROTOCOL.Read() function may be costly.
Read both file operands in chunks of "PcdShellFileOperationSize" bytes.
Draw bytes for comparison from the internal read-ahead buffers.
Some ad-hoc measurements on my laptop, using OVMF, and the 4KB default of
"PcdShellFileOperationSize":
- When comparing two identical 1MB files that are served by EnhancedFatDxe
on top of VirtioScsiDxe, this patch brings no noticeable improvement;
the comparison completes in <1s both before and after.
- When comparing two identical 1MB files served by VirtioFsDxe, the
comparison time improves from 2 minutes 25 seconds to <1s.
Cc: Philippe Mathieu-Daudé <philmd at redhat.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Zhichao Gao <zhichao.gao at intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3123
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf | 1 +
ShellPkg/Library/UefiShellDebug1CommandsLib/Comp.c | 127 +++++++++++++++++++-
2 files changed, 125 insertions(+), 3 deletions(-)
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
index ed94477a0642..74ad5facf6b1 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
@@ -113,6 +113,7 @@ [LibraryClasses]
BcfgCommandLib
[Pcd]
+ gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize ## CONSUMES
gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES
[Protocols]
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Comp.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Comp.c
index 0a5d13f01c7b..0df0b3149369 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Comp.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Comp.c
@@ -21,6 +21,16 @@ typedef enum {
InPrevDiffPoint
} READ_STATUS;
+//
+// Buffer type, for reading both file operands in chunks.
+//
+typedef struct {
+ UINT8 *Data; // dynamically allocated buffer
+ UINTN Allocated; // the allocated size of Data
+ UINTN Next; // next position in Data to fetch a byte at
+ UINTN Left; // number of bytes left in Data for fetching at Next
+} FILE_BUFFER;
+
/**
Function to print differnt point data.
@@ -76,6 +86,106 @@ PrintDifferentPoint(
ShellPrintEx (-1, -1, L"*\r\n");
}
+/**
+ Initialize a FILE_BUFFER.
+
+ @param[out] FileBuffer The FILE_BUFFER to initialize. On return, the caller
+ is responsible for checking FileBuffer->Data: if
+ FileBuffer->Data is NULL on output, then memory
+ allocation failed.
+**/
+STATIC
+VOID
+FileBufferInit (
+ OUT FILE_BUFFER *FileBuffer
+ )
+{
+ FileBuffer->Allocated = PcdGet32 (PcdShellFileOperationSize);
+ FileBuffer->Data = AllocatePool (FileBuffer->Allocated);
+ FileBuffer->Left = 0;
+}
+
+/**
+ Uninitialize a FILE_BUFFER.
+
+ @param[in,out] FileBuffer The FILE_BUFFER to uninitialize. The caller is
+ responsible for making sure FileBuffer was first
+ initialized with FileBufferInit(), successfully or
+ unsuccessfully.
+**/
+STATIC
+VOID
+FileBufferUninit (
+ IN OUT FILE_BUFFER *FileBuffer
+ )
+{
+ SHELL_FREE_NON_NULL (FileBuffer->Data);
+}
+
+/**
+ Read a byte from a SHELL_FILE_HANDLE, buffered with a FILE_BUFFER.
+
+ @param[in] FileHandle The SHELL_FILE_HANDLE to replenish FileBuffer
+ from, if needed.
+
+ @param[in,out] FileBuffer The FILE_BUFFER to read a byte from. If FileBuffer
+ is empty on entry, then FileBuffer is refilled
+ from FileHandle, before outputting a byte from
+ FileBuffer to Byte. The caller is responsible for
+ ensuring that FileBuffer was successfully
+ initialized with FileBufferInit().
+
+ @param[out] BytesRead On successful return, BytesRead is set to 1 if the
+ next byte from FileBuffer has been stored to Byte.
+ On successful return, BytesRead is set to 0 if
+ FileBuffer is empty, and FileHandle is at EOF.
+ When an error is returned, BytesRead is not set.
+
+ @param[out] Byte On output, the next byte from FileBuffer. Only set
+ if (a) EFI_SUCCESS is returned and (b) BytesRead
+ is set to 1 on output.
+
+ @retval EFI_SUCCESS BytesRead has been set to 0 or 1. In the latter case,
+ Byte has been set as well.
+
+ @return Error codes propagated from
+ gEfiShellProtocol->ReadFile().
+**/
+STATIC
+EFI_STATUS
+FileBufferReadByte (
+ IN SHELL_FILE_HANDLE FileHandle,
+ IN OUT FILE_BUFFER *FileBuffer,
+ OUT UINTN *BytesRead,
+ OUT UINT8 *Byte
+ )
+{
+ UINTN ReadSize;
+ EFI_STATUS Status;
+
+ if (FileBuffer->Left == 0) {
+ ReadSize = FileBuffer->Allocated;
+ Status = gEfiShellProtocol->ReadFile (FileHandle, &ReadSize,
+ FileBuffer->Data);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ if (ReadSize == 0) {
+ *BytesRead = 0;
+ return EFI_SUCCESS;
+ }
+ FileBuffer->Next = 0;
+ FileBuffer->Left = ReadSize;
+ }
+
+ *BytesRead = 1;
+ *Byte = FileBuffer->Data[FileBuffer->Next];
+
+ FileBuffer->Next++;
+ FileBuffer->Left--;
+ return EFI_SUCCESS;
+}
+
/**
Function for 'comp' command.
@@ -107,6 +217,8 @@ ShellCommandRunComp (
UINT8 OneByteFromFile2;
UINT8 *DataFromFile1;
UINT8 *DataFromFile2;
+ FILE_BUFFER FileBuffer1;
+ FILE_BUFFER FileBuffer2;
UINTN InsertPosition1;
UINTN InsertPosition2;
UINTN DataSizeFromFile1;
@@ -234,10 +346,15 @@ ShellCommandRunComp (
if (ShellStatus == SHELL_SUCCESS) {
DataFromFile1 = AllocateZeroPool ((UINTN)DifferentBytes);
DataFromFile2 = AllocateZeroPool ((UINTN)DifferentBytes);
- if (DataFromFile1 == NULL || DataFromFile2 == NULL) {
+ FileBufferInit (&FileBuffer1);
+ FileBufferInit (&FileBuffer2);
+ if (DataFromFile1 == NULL || DataFromFile2 == NULL ||
+ FileBuffer1.Data == NULL || FileBuffer2.Data == NULL) {
ShellStatus = SHELL_OUT_OF_RESOURCES;
SHELL_FREE_NON_NULL (DataFromFile1);
SHELL_FREE_NON_NULL (DataFromFile2);
+ FileBufferUninit (&FileBuffer1);
+ FileBufferUninit (&FileBuffer2);
}
}
@@ -247,9 +364,11 @@ ShellCommandRunComp (
DataSizeFromFile2 = 1;
OneByteFromFile1 = 0;
OneByteFromFile2 = 0;
- Status = gEfiShellProtocol->ReadFile (FileHandle1, &DataSizeFromFile1, &OneByteFromFile1);
+ Status = FileBufferReadByte (FileHandle1, &FileBuffer1,
+ &DataSizeFromFile1, &OneByteFromFile1);
ASSERT_EFI_ERROR (Status);
- Status = gEfiShellProtocol->ReadFile (FileHandle2, &DataSizeFromFile2, &OneByteFromFile2);
+ Status = FileBufferReadByte (FileHandle2, &FileBuffer2,
+ &DataSizeFromFile2, &OneByteFromFile2);
ASSERT_EFI_ERROR (Status);
TempAddress++;
@@ -346,6 +465,8 @@ ShellCommandRunComp (
SHELL_FREE_NON_NULL (DataFromFile1);
SHELL_FREE_NON_NULL (DataFromFile2);
+ FileBufferUninit (&FileBuffer1);
+ FileBufferUninit (&FileBuffer2);
if (DiffPointNumber == 0) {
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_COMP_FOOTER_PASS), gShellDebug1HiiHandle);
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69587): https://edk2.groups.io/g/devel/message/69587
Mute This Topic: https://groups.io/mt/79426445/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list