dest.Append(c);

FYI, building the string up one at a time is really slow... (I know, not new code).

I was curious how slow, so I came up with four alternatives:

• Append One Char at a time - current approach
• Append All or One at a time - check for quote, then pick Append String or Char
• Append Incrementally - run through the string char for char, but append in chunks
• Append All or Incrementally - check for quote, then chunk

For test data without quotes, the times are:

• Append One Char at a time: 374
• Append All or One at a time: 163
• Append Incrementally: 331
• Append All or Incrementally: 163

In other words, it really pays off (2x faster) to add the check (if indexof('"') == -1 then append string). But what if the string has a quote (or more)? How much time does it add?

For test data with a quote, the times are:

• Append One Char at a time: 382
• Append All or One at a time: 404
• Append Incrementally: 348
• Append All or Incrementally: 379

So adding the extra check added ~6% overhead in this case. (which we can make back up by using an incremental chunked approach). Probably worth it since the bulk of strings probably won't have embedded quotes, so we'll probably average 2x faster.

Refers to: src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs:1081 in fe379e2. [](commit_id = fe379e2, deletion_comment = False)

            dest.Append(c);

FYI, here's the optimized code:

        private static void AddStringAllOrIncremental(StringBuilder dest, string source)
        {
            if (source == null)
            {
                return;
            }

            // Adding Double quote to all strings
            dest.Append('"');
            if (source.IndexOf('"') >= 0) // shortcut if escape not necessary
            {
                int startIndex = 0;
                for (int i = 0; i < source.Length; i++)
                {
                    char c = source[i];

                    // Double quote in the string is escaped with double quote
                    if (c == '"')
                    {
                        dest.Append(source, startIndex, i - startIndex);  // copy everything up to this point
                        dest.Append('"');
                        startIndex = i;
                    }
                }
                dest.Append(source, startIndex, source.Length - startIndex); // copy the rest of the string
            }
            else
            {
                dest.Append(source);
            }

            dest.Append('"');
        }

    }

In reply to: 466544670 [](ancestors = 466544670)

Refers to: src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs:1081 in fe379e2. [](commit_id = fe379e2, deletion_comment = False)

            dest.Append(c);

In fairness, might be better as a separate change, since it's an optimization of existing code, only indirectly related to the new code.

In reply to: 466545320 [](ancestors = 466545320,466544670)

Refers to: src/Microsoft.PowerShell.Commands.Utility/commands/utility/CSVCommands.cs:1081 in fe379e2. [](commit_id = fe379e2, deletion_comment = False)

@DavidBerg-MSFT Many thanks! It is very interesting!
I hope the PR will fast reviewed and we get the new, very useful feature in 6.2 version. Then we could work on optimizations (also we plan to move to .Net Core 3.0 after releasing 6.2 that give us performance benefits too).

@SteveL-MSFT If your team is interesting to have this in 6.2 please review.

@DavidBerg-MSFT agree that the code optimization should be a separate PR.

@iSazonov can you address the Codacy issues?

Having taken a look at the code, the risk is small so we may be able to take this for 6.2-GA, but it won't make it into RC.

can you address the Codacy issues?

Done.

@iSazonov Looks like there is still 2 minor Codacy updates needed.

@anmenaga First was fixed. Second is false positive.

@SteveL-MSFT Will this be added to 6.2-GA?

Can you ask this in the maintainers teams channel with a link to the PR with your reason why and an analysis of the regression risk?

@TravisEz13 This is only an informational question to the previous Steve's comment. I don't need to have it in 6.2.

We have already forked for 6.2-GA based no RC.1. So, this won't be in the build.

@DavidBerg-MSFT I tested your code on .Net Core 3.0 Preview3 and can not confirm results. In my tests original code is faster.
What .Net Core did you use? Could you share code of your tests?

I looked the code again. StringBuilder delays only on reallocating buffers. We reuse StringBuilder and exclude such re-allocations. Underlying method in StringBuilder copies very efficient by means of indexer for both char and strings. So I can not find how we could speed up the code - IndexOf() slow down the code and my tests confirm this.