ShellPkg/Comp: add file buffering

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@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3123
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Message-Id: <20210113085453.10168-2-lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit is contained in:
Laszlo Ersek 2021-01-13 09:54:44 +01:00 committed by mergify[bot]
parent e68c2a22ca
commit f72fd9616e
2 changed files with 125 additions and 3 deletions

View File

@ -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);

View File

@ -113,6 +113,7 @@
BcfgCommandLib
[Pcd]
gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize ## CONSUMES
gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES
[Protocols]