Conversation
Reviewer's GuideThis PR refactors the CSS bundler to use streamed byte-level I/O with a pooled 64KB buffer, skips UTF-8 BOM on the first chunk, and ensures the rented buffer is returned, boosting performance and reducing memory pressure. Class diagram for updated Bundler logicclassDiagram
class BundlerOptions {
+string OutputFileName
+List<string> InputFiles
}
class Bundler {
+static void DoBundler(string bundlerFile, BundlerOptions option)
}
Bundler --> BundlerOptions: uses
Flow diagram for new streaming bundling processflowchart TD
A[Start Bundling] --> B[Rent buffer from ArrayPool]
B --> C[Open output file for writing]
C --> D[For each input file]
D --> E{File exists?}
E -- No --> D
E -- Yes --> F[Open input file for reading]
F --> G[Read chunk into buffer]
G --> H{First chunk has BOM?}
H -- Yes --> I[Skip BOM and write rest to output]
H -- No --> J[Write chunk to output]
I --> K{More data?}
J --> K
K -- Yes --> G
K -- No --> D
D --> L[Close output file]
L --> M[Return buffer to ArrayPool]
M --> N[End Bundling]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:82-85` </location>
<code_context>
- var content = reader.ReadToEnd();
- writer.Write(Encoding.UTF8.GetBytes(content));
- reader.Close();
+ if (reader.Position == read && read >= 3 && buffer[0] == 0xEF && buffer[1] == 0xBB && buffer[2] == 0xBF)
+ {
+ // skip bom
+ writer.Write(buffer, 3, read - 3);
+ continue;
+ }
</code_context>
<issue_to_address>
**issue:** BOM detection logic may not work for files larger than the buffer or with BOM not at the start.
Check for BOM only on the initial read and ensure it is handled once, as the current approach may miss BOMs in larger files or if not at the very start.
</issue_to_address>
### Comment 2
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:61` </location>
<code_context>
- using var writer = File.OpenWrite(Path.Combine(rootFolder, option.OutputFileName));
- foreach (var file in option.InputFiles)
+ var buffer = ArrayPool<byte>.Shared.Rent(64 * 1024 * 1024);
+ try
{
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing manual buffer management with built-in text APIs to simplify BOM handling and file concatenation.
Consider dropping the manual buffer/rent-return logic and just using the built-in text APIs (they handle BOM detection/stripping for you). For example, you can replace the entire `ArrayPool` + read/write loop with something like:
```csharp
var outPath = Path.Combine(rootFolder, option.OutputFileName);
// new UTF8Encoding(false) => no BOM on the *output*
using var writer = new StreamWriter(outPath, append: false, encoding: new UTF8Encoding(false));
foreach (var file in option.InputFiles)
{
var inPath = Path.Combine(rootFolder, file);
if (!File.Exists(inPath)) continue;
// File.ReadAllText auto-detects & strips BOM
var text = File.ReadAllText(inPath);
writer.Write(text);
}
```
If you’re concerned about peak memory for very large files, you can stream each file line-by-line instead:
```csharp
using var writer = new StreamWriter(outPath, false, new UTF8Encoding(false));
foreach (var file in option.InputFiles)
{
var inPath = Path.Combine(rootFolder, file);
if (!File.Exists(inPath)) continue;
foreach (var line in File.ReadLines(inPath))
writer.WriteLine(line);
}
```
Both approaches
- remove `ArrayPool` complexity
- preserve all original functionality (concatenation, BOM stripping)
- are far more readable/maintainable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the CSS bundler to improve performance by using ArrayPool<byte> for buffer management instead of reading files as text. The changes include switching from text-based to binary-based file reading with UTF-8 BOM detection and removal.
Key changes:
- Replaced
System.TextwithSystem.Buffersto enable ArrayPool usage - Changed from
File.OpenTexttoFile.OpenReadwith manual byte buffer reading - Added BOM detection logic to skip UTF-8 BOM markers (0xEF, 0xBB, 0xBF) from file starts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@sourcery-ai review |
SourceryAI
reviewed
Nov 10, 2025
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Use File.Create or File.Open with FileMode.Create to ensure the output file is truncated rather than overwritten partially by File.OpenWrite.
- The explicit writer.Close() is redundant when using a 'using var writer'; you can remove those calls and rely on disposal to close the stream.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Use File.Create or File.Open with FileMode.Create to ensure the output file is truncated rather than overwritten partially by File.OpenWrite.
- The explicit writer.Close() is redundant when using a 'using var writer'; you can remove those calls and rely on disposal to close the stream.
## Individual Comments
### Comment 1
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:65` </location>
<code_context>
+ try
{
- var inputFile = Path.Combine(rootFolder, file);
- if (!File.Exists(inputFile))
+ using var writer = File.OpenWrite(Path.Combine(rootFolder, option.OutputFileName));
+ foreach (var file in option.InputFiles)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Silent skipping of missing input files may hide errors.
Consider adding a warning or error log when an input file is missing to improve traceability and help identify configuration issues.
</issue_to_address>
### Comment 2
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:90` </location>
<code_context>
+ writer.Write(buffer, 0, read);
+ }
+ }
+ writer.Close();
+ }
+ finally
</code_context>
<issue_to_address>
**nitpick:** Explicit writer.Close() is redundant with using statement.
Removing the explicit writer.Close() will streamline the code and prevent confusion, as the using statement already handles resource disposal.
</issue_to_address>
### Comment 3
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:61` </location>
<code_context>
- using var writer = File.OpenWrite(Path.Combine(rootFolder, option.OutputFileName));
- foreach (var file in option.InputFiles)
+ var buffer = ArrayPool<byte>.Shared.Rent(64 * 1024);
+ try
{
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing manual buffer management and BOM handling with Stream.CopyTo and a small helper method to simplify the code.
Here’s one way to get rid of all the manual pooling, loops, BOM-checks and extra scopes by using Stream.CopyTo (and a tiny helper to skip the UTF8-BOM). This does exactly what you have today, but in ~10 lines instead of 50+:
```csharp
var outputPath = Path.Combine(rootFolder, option.OutputFileName);
using var output = File.Create(outputPath);
foreach (var file in option.InputFiles)
{
var inputPath = Path.Combine(rootFolder, file);
if (!File.Exists(inputPath)) continue;
using var input = File.OpenRead(inputPath);
// Try skip BOM, otherwise rewind back to start
if (TrySkipUtf8Bom(input) == false)
input.Seek(0, SeekOrigin.Begin);
// Copy everything (with a single internal buffer) directly into the writer
input.CopyTo(output);
}
Console.WriteLine($"Bundler Completed .... {option.OutputFileName}");
// ----------------------------------------------------------------
static bool TrySkipUtf8Bom(Stream s)
{
Span<byte> buf = stackalloc byte[3];
if (s.Read(buf) != buf.Length) return false;
return buf.SequenceEqual(Encoding.UTF8.GetPreamble());
}
```
What changed:
- Remove `ArrayPool<byte>` + manual `while`‐loops – `Stream.CopyTo` does that for you.
- A tiny helper to peek & skip the BOM, or rewind if no BOM.
- One `using var output` + one `using var input` per file; no nested `try/finally` or explicit `Close()` calls.
</issue_to_address>Hi @ArgoZhang! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Use File.Create or File.Open with FileMode.Create for the output stream to ensure the file is truncated and avoid leftover bytes from previous runs.
- Remove the explicit writer.Close() calls as the using declaration will automatically dispose and close the stream at the end of the scope.
- Extract the BOM-skipping and chunked copy logic into a dedicated helper method to simplify DoBundler and improve overall readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Use File.Create or File.Open with FileMode.Create for the output stream to ensure the file is truncated and avoid leftover bytes from previous runs.
- Remove the explicit writer.Close() calls as the using declaration will automatically dispose and close the stream at the end of the scope.
- Extract the BOM-skipping and chunked copy logic into a dedicated helper method to simplify DoBundler and improve overall readability.
## Individual Comments
### Comment 1
<location> `tools/BootstrapBlazor.CssBundler/Bundler.cs:61` </location>
<code_context>
- using var writer = File.OpenWrite(Path.Combine(rootFolder, option.OutputFileName));
- foreach (var file in option.InputFiles)
+ var buffer = ArrayPool<byte>.Shared.Rent(64 * 1024);
+ try
{
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing manual buffer management and loops with a simpler BOM check and CopyTo for file streaming.
Here’s a way to keep exactly the same BOM‐skipping behavior but drop all of the manual buffer‐renting, loops, and try/finally. We just peek the first three bytes once, adjust `Position`, then call `CopyTo`:
```csharp
using var writer = File.OpenWrite(Path.Combine(rootFolder, option.OutputFileName));
foreach (var file in option.InputFiles)
{
var inputFile = Path.Combine(rootFolder, file);
if (!File.Exists(inputFile)) continue;
using var fs = File.OpenRead(inputFile);
if (fs.Length >= 3)
{
Span<byte> preamble = stackalloc byte[3];
if (fs.Read(preamble) == 3
&& preamble[0] == 0xEF
&& preamble[1] == 0xBB
&& preamble[2] == 0xBF)
{
// BOM detected – stream position is already at 3
}
else
{
fs.Position = 0; // no BOM, rewind
}
}
fs.CopyTo(writer, bufferSize: 64 * 1024);
}
Console.WriteLine($"Bundler Completed .... {option.OutputFileName}");
```
Steps:
1. Remove the `ArrayPool<byte>` rent/return and explicit `while` loop.
2. Use `fs.CopyTo(writer, 64*1024)` to stream the rest.
3. Detect BOM by reading just 3 bytes into a stack buffer and rewinding if it’s not there.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #668
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Improve the bundler by switching to a stream-based implementation using ArrayPool buffers and handling UTF8 BOM correctly to boost performance and ensure clean output
Bug Fixes:
Enhancements: