Bryan Berns (@NoMoreFood) Please add a test.

Ilya (@iSazonov) Since this addresses an internal performance issue, did you just want a textual output posted here with before/after performance tests for a variety of text encodings?

Bryan Berns (@NoMoreFood) I do not look the issue in depths but I feel that if UTF-8 detection wrong the cmdlet can return wrong results. In the case we should add tests.

Ilya (@iSazonov) Alright, I'll take a look into creating a general, functional test. I will say though that this was not an issue on Windows PowerShell. And notably, Windows PowerShell was not failing the _currentEncoding.Equals(Encoding.UTF8) comparison that resulted in this problem.

I imagine there may have been a change in the .NET Core API at some point that resulted in that failing. It seems on the surface like a relatively innocuous change until you start doing reference equality comparisons against it for things like this. 😁

Bryan Berns (@NoMoreFood) Please check that your new tests fail on current version and do not fail after your fix. Thanks!

Ilya (@iSazonov) It appears there are already functional tests for various encodings within the test suite. However, I added a few additional explicit encodings that were not in the enumeration and committed that change to this pull. Given this pull addresses a performance issue, I do not see how a reliable pass/fail test can be written given performance can vary drastically between systems and, to a lesser extent, between executions on the same system given other operating system activity. The only reasonable way to provide a test for something like this would be to add debug/tracing code to the code base to verify the desired code branch is hit/not hit; I do not see any precedent for this type of testing in other tests.

In absence of that, I have provided a before and after demonstration that clearly shows the performance change on my system.

Test Code:

$Encodings = @('String','OEM','Unicode','BigEndianUnicode',
    'UTF8','UTF8BOM','UTF8NoBOM','UTF7','UTF32','Ascii')
$TempFile = (New-TemporaryFile).FullName
$Results = @()
ForEach ($Encoding in $Encodings)
{
    (1..2e6) | Set-Content -Encoding $Encoding -LiteralPath $TempFile -Force
    $Time = Measure-Command { $Capture = Get-Content -Tail 1 -LiteralPath $TempFile }
    $Results += New-Object PSObject -Property @{'Encoding'=$Encoding;'Time'=$Time.TotalMilliSeconds}
    Remove-Item -LiteralPath $TempFile -Force
}
$Results | Format-Table -AutoSize

Before Changes:

Encoding             Time
--------             ----
String            12.3904
OEM              585.5496
Unicode            1.2809
BigEndianUnicode   1.1258
UTF8             555.5736
UTF8BOM            0.9639
UTF8NoBOM        647.7248
UTF7             560.4215
UTF32              1.1847
Ascii            562.1341

After Changes:

Encoding           Time
--------           ----
String           2.8139
OEM              0.8078
Unicode          0.8523
BigEndianUnicode 0.9938
UTF8             0.7483
UTF8BOM          0.7559
UTF8NoBOM        0.7721
UTF7             0.8044
UTF32            1.1415
Ascii            0.9463

Notice the difference the timings for the code that will be detected as UTF-8 (given the current character set used in the demonstration). The "after" results are similar to what you would see with Windows PowerShell.

Bryan Berns (@NoMoreFood) I guess the updated test does not fail on current version. Can you check? If so I suggest replace the test text (really it is one byte ASCII)

        $content = @"
one
two
foo
bar
baz
"@

with a text having multi byte Unicode, ex.:

        $content = @"
один
два
фуу
бар
база
"@

Ilya (@iSazonov) Other parts of that test case will need to change as well to support multi-byte Unicode validation. It'll beef it up later today and submit an update. On the surface after a quick mod, it does not appear to behave any differently before or after these changes (except for the fact it's faster). More to come...

Ilya (@iSazonov) Enhanced Get-Content / Get-Content -Tail tests have been added.

Bryan Berns (@NoMoreFood) Is there an issue filed for this fix? If not, can you please open an issue that describes the problem with repro steps?

Dongbo Wang (@daxian-dbw) Yes, it's in the PR description: #11830

Oh, I see. Maybe that part of the template is not clear enough :) That's for the doc issue if a documentation change is needed

The issue which the PR is addressing should be put in the PR description, like Fix #xxxx. I added it to your PR description.

Bryan Berns (@NoMoreFood) Thanks for your contribution!

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes