Skip to content

Improve PaxTarEntry construction performance #132013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rzikm wants to merge 1 commit into
base: main
Choose a base branch
from
+139 −40
Open

Improve PaxTarEntry construction performance #132013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 97 additions & 21 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,65 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable<KeyValuePair<strin
return;
}

AddExtendedAttributes(extendedAttributes);
ExtendedAttributeValues values = default;
using IEnumerator<KeyValuePair<string, string>> enumerator = extendedAttributes.GetEnumerator();
while (enumerator.MoveNext())
{
KeyValuePair<string, string> extendedAttribute = enumerator.Current;

ValidateExtendedAttribute(extendedAttribute);
_ea ??= new Dictionary<string, string>();
_ea.Add(extendedAttribute.Key, extendedAttribute.Value);

switch (extendedAttribute.Key)
{
case PaxEaName:
values.Name = extendedAttribute.Value;
break;
case PaxEaLinkName:
values.LinkName = extendedAttribute.Value;
break;
case PaxEaMTime:
values.MTime = extendedAttribute.Value;
break;
case PaxEaMode:
values.Mode = extendedAttribute.Value;
break;
case PaxEaSize:
values.Size = extendedAttribute.Value;
break;
case PaxEaGnuSparseName:
values.GnuSparseName = extendedAttribute.Value;
break;
case PaxEaGnuSparseRealSize:
values.GnuSparseRealSize = extendedAttribute.Value;
break;
case PaxEaGnuSparseMajor:
values.GnuSparseMajor = extendedAttribute.Value;
break;
case PaxEaGnuSparseMinor:
values.GnuSparseMinor = extendedAttribute.Value;
break;
case PaxEaUid:
values.Uid = extendedAttribute.Value;
break;
case PaxEaGid:
values.Gid = extendedAttribute.Value;
break;
case PaxEaUName:
values.UName = extendedAttribute.Value;
break;
case PaxEaGName:
values.GName = extendedAttribute.Value;
break;
case PaxEaDevMajor:
values.DevMajor = extendedAttribute.Value;
break;
case PaxEaDevMinor:
values.DevMinor = extendedAttribute.Value;
break;
}
}

if (_ea == null || _ea.Count == 0)
{
Expand All @@ -107,33 +165,33 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable<KeyValuePair<strin
// as-is to avoid data loss during roundtripping.

// The 'name' header field only fits 100 bytes, so we always store the full name text to the dictionary.
if (ExtendedAttributes.TryGetValue(PaxEaName, out string? paxEaName))
if (values.Name is not null)
{
_name = paxEaName;
_name = values.Name;
}

// The 'linkName' header field only fits 100 bytes, so we always store the full linkName text to the dictionary.
// Only apply to link entries to avoid setting _linkName on non-link entry types.
if (_typeFlag is TarEntryType.HardLink or TarEntryType.SymbolicLink &&
ExtendedAttributes.TryGetValue(PaxEaLinkName, out string? paxEaLinkName))
values.LinkName is not null)
{
_linkName = paxEaLinkName;
_linkName = values.LinkName;
}

// The 'mtime' header field only fits 12 bytes, so a more precise timestamp goes in the extended attributes
if (TarHelpers.TryGetDateTimeOffsetFromTimestampString(ExtendedAttributes, PaxEaMTime, out DateTimeOffset mTime))
if (TarHelpers.TryGetDateTimeOffsetFromTimestampString(values.MTime, out DateTimeOffset mTime))
{
_mTime = mTime;
}

// The user could've stored an override in the extended attributes
if (TarHelpers.TryGetStringAsBaseTenInteger(ExtendedAttributes, PaxEaMode, out int mode))
if (TarHelpers.TryGetStringAsBaseTenInteger(values.Mode, out int mode))
{
_mode = mode;
}

// The 'size' header field only fits 12 bytes, so the data section length that surpases that limit needs to be retrieved
if (TarHelpers.TryGetStringAsBaseTenLong(ExtendedAttributes, PaxEaSize, out long size))
if (TarHelpers.TryGetStringAsBaseTenLong(values.Size, out long size))
{
if (size < 0)
{
Expand All @@ -148,14 +206,14 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable<KeyValuePair<strin
if (_typeFlag is TarEntryType.RegularFile or TarEntryType.V7RegularFile)
{
// 'GNU.sparse.name' overrides the placeholder path (e.g. 'GNUSparseFile.0/...') in the header's 'path' field.
if (ExtendedAttributes.TryGetValue(PaxEaGnuSparseName, out string? gnuSparseName))
if (values.GnuSparseName is not null)
{
_name = gnuSparseName;
_name = values.GnuSparseName;
}

// 'GNU.sparse.realsize' is the expanded (virtual) file size; stored separately from _size so that
// _size retains the archive data section length needed for correct stream positioning.
if (TarHelpers.TryGetStringAsBaseTenLong(ExtendedAttributes, PaxEaGnuSparseRealSize, out long gnuSparseRealSize))
if (TarHelpers.TryGetStringAsBaseTenLong(values.GnuSparseRealSize, out long gnuSparseRealSize))
{
if (gnuSparseRealSize < 0)
{
Expand All @@ -166,50 +224,68 @@ internal void ReplaceNormalAttributesWithExtended(IEnumerable<KeyValuePair<strin

// 'GNU.sparse.major=1' and 'GNU.sparse.minor=0' identify format 1.0, where the data section begins
// with an embedded text-format sparse map followed by the packed non-zero data segments.
if (ExtendedAttributes.TryGetValue(PaxEaGnuSparseMajor, out string? gnuSparseMajor) && gnuSparseMajor == "1" &&
ExtendedAttributes.TryGetValue(PaxEaGnuSparseMinor, out string? gnuSparseMinor) && gnuSparseMinor == "0")
if (values.GnuSparseMajor == "1" && values.GnuSparseMinor == "0")
{
_isGnuSparse10 = true;
}
}

// The 'uid' header field only fits 8 bytes, or the user could've stored an override in the extended attributes
if (TarHelpers.TryGetStringAsBaseTenInteger(ExtendedAttributes, PaxEaUid, out int uid))
if (TarHelpers.TryGetStringAsBaseTenInteger(values.Uid, out int uid))
{
_uid = uid;
}

// The 'gid' header field only fits 8 bytes, or the user could've stored an override in the extended attributes
if (TarHelpers.TryGetStringAsBaseTenInteger(ExtendedAttributes, PaxEaGid, out int gid))
if (TarHelpers.TryGetStringAsBaseTenInteger(values.Gid, out int gid))
{
_gid = gid;
}

// The 'uname' header field only fits 32 bytes
if (ExtendedAttributes.TryGetValue(PaxEaUName, out string? paxEaUName))
if (values.UName is not null)
{
_uName = paxEaUName;
_uName = values.UName;
}

// The 'gname' header field only fits 32 bytes
if (ExtendedAttributes.TryGetValue(PaxEaGName, out string? paxEaGName))
if (values.GName is not null)
{
_gName = paxEaGName;
_gName = values.GName;
}

// The 'devmajor' header field only fits 8 bytes, or the user could've stored an override in the extended attributes
if (TarHelpers.TryGetStringAsBaseTenInteger(ExtendedAttributes, PaxEaDevMajor, out int devMajor))
if (TarHelpers.TryGetStringAsBaseTenInteger(values.DevMajor, out int devMajor))
{
_devMajor = devMajor;
}

// The 'devminor' header field only fits 8 bytes, or the user could've stored an override in the extended attributes
if (TarHelpers.TryGetStringAsBaseTenInteger(ExtendedAttributes, PaxEaDevMinor, out int devMinor))
if (TarHelpers.TryGetStringAsBaseTenInteger(values.DevMinor, out int devMinor))
{
_devMinor = devMinor;
}
}

private struct ExtendedAttributeValues
{
internal string? Name;
internal string? LinkName;
internal string? MTime;
internal string? Mode;
internal string? Size;
internal string? GnuSparseName;
internal string? GnuSparseRealSize;
internal string? GnuSparseMajor;
internal string? GnuSparseMinor;
internal string? Uid;
internal string? Gid;
internal string? UName;
internal string? GName;
internal string? DevMajor;
internal string? DevMinor;
}

// Determines what kind of stream needs to be saved for the data section.
// - Metadata typeflag entries (Extended Attributes and Global Extended Attributes in PAX, LongLink and LongPath in GNU)
// will get all the data section read and the stream pointer positioned at the beginning of the next header.
Expand Down
25 changes: 14 additions & 11 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,25 @@ internal void AddExtendedAttributes(IEnumerable<KeyValuePair<string, string>> ex
{
KeyValuePair<string, string> kvp = enumerator.Current;

int index = kvp.Key.AsSpan().IndexOfAny('=', '\n');
if (index >= 0)
{
throw new ArgumentException(SR.Format(SR.TarExtAttrDisallowedKeyChar, kvp.Key, kvp.Key[index] == '\n' ? "\\n" : kvp.Key[index]));
}
if (kvp.Value.Contains('\n'))
{
throw new ArgumentException(SR.Format(SR.TarExtAttrDisallowedValueChar, kvp.Key, "\\n"));
}

ValidateExtendedAttribute(kvp);
_ea ??= new Dictionary<string, string>();

_ea.Add(kvp.Key, kvp.Value);
}
}

private static void ValidateExtendedAttribute(KeyValuePair<string, string> extendedAttribute)
{
int index = extendedAttribute.Key.AsSpan().IndexOfAny('=', '\n');
if (index >= 0)
{
throw new ArgumentException(SR.Format(SR.TarExtAttrDisallowedKeyChar, extendedAttribute.Key, extendedAttribute.Key[index] == '\n' ? "\\n" : extendedAttribute.Key[index]));
}
if (extendedAttribute.Value.Contains('\n'))
{
throw new ArgumentException(SR.Format(SR.TarExtAttrDisallowedValueChar, extendedAttribute.Key, "\\n"));
}
}

private static string GetMagicForFormat(TarEntryFormat format) => format switch
{
TarEntryFormat.Ustar or TarEntryFormat.Pax => UstarMagic,
Expand Down
36 changes: 28 additions & 8 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,20 @@ private static decimal GetSecondsSinceEpochFromDateTimeOffset(DateTimeOffset dat
// If the specified fieldName is found in the provided dictionary and it is a valid decimal number, returns true and sets the value in 'dateTimeOffset'.
internal static bool TryGetDateTimeOffsetFromTimestampString(Dictionary<string, string>? dict, string fieldName, out DateTimeOffset dateTimeOffset)
{
dateTimeOffset = default;
if (dict != null &&
dict.TryGetValue(fieldName, out string? value) &&
decimal.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out decimal secondsSinceEpoch))
return TryGetDateTimeOffsetFromTimestampString(
dict is not null && dict.TryGetValue(fieldName, out string? value) ? value : null,
out dateTimeOffset);
}

internal static bool TryGetDateTimeOffsetFromTimestampString(string? value, out DateTimeOffset dateTimeOffset)
{
if (decimal.TryParse(value, NumberStyles.Any, CultureInfo.InvariantCulture, out decimal secondsSinceEpoch))
{
dateTimeOffset = GetDateTimeOffsetFromSecondsSinceEpoch(secondsSinceEpoch);
return true;
}

dateTimeOffset = default;
return false;
}

Expand All @@ -151,9 +157,16 @@ internal static string GetTimestampStringFromDateTimeOffset(DateTimeOffset times
// If the specified fieldName is found in the provided dictionary and is a valid string representation of a number, returns true and sets the value in 'baseTenInteger'.
internal static bool TryGetStringAsBaseTenInteger(IReadOnlyDictionary<string, string> dict, string fieldName, out int baseTenInteger)
{
if (dict.TryGetValue(fieldName, out string? strNumber) && !string.IsNullOrEmpty(strNumber))
return TryGetStringAsBaseTenInteger(
dict.TryGetValue(fieldName, out string? value) ? value : null,
out baseTenInteger);
}

internal static bool TryGetStringAsBaseTenInteger(string? value, out int baseTenInteger)
{
if (!string.IsNullOrEmpty(value))
{
baseTenInteger = int.Parse(strNumber, CultureInfo.InvariantCulture);
baseTenInteger = int.Parse(value, CultureInfo.InvariantCulture);
return true;
}

Expand All @@ -164,9 +177,16 @@ internal static bool TryGetStringAsBaseTenInteger(IReadOnlyDictionary<string, st
// If the specified fieldName is found in the provided dictionary and is a valid string representation of a number, returns true and sets the value in 'baseTenLong'.
internal static bool TryGetStringAsBaseTenLong(IReadOnlyDictionary<string, string> dict, string fieldName, out long baseTenLong)
{
if (dict.TryGetValue(fieldName, out string? strNumber) && !string.IsNullOrEmpty(strNumber))
return TryGetStringAsBaseTenLong(
dict.TryGetValue(fieldName, out string? value) ? value : null,
out baseTenLong);
}

internal static bool TryGetStringAsBaseTenLong(string? value, out long baseTenLong)
{
if (!string.IsNullOrEmpty(value))
{
baseTenLong = long.Parse(strNumber, CultureInfo.InvariantCulture);
baseTenLong = long.Parse(value, CultureInfo.InvariantCulture);
return true;
}

Expand Down
Loading