Skip to content

Conversation

@jamescourtney
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Apr 27, 2025

Codecov Report

Attention: Patch coverage is 66.34199% with 311 lines in your changes missing coverage. Please review.

Project coverage is 92.92%. Comparing base (eacf325) to head (6a9e2be).

Files with missing lines Patch % Lines
...p.Runtime/IO/SerializationTarget/BigSpanHelpers.cs 32.05% 201 Missing and 30 partials ⚠️
...latSharp.Runtime/IO/SerializationTarget/BigSpan.cs 72.22% 32 Missing and 3 partials ⚠️
....Runtime/IO/SerializationTarget/BigReadOnlySpan.cs 38.88% 11 Missing ⚠️
src/FlatSharp.Runtime/ISerializerExtensions.cs 50.00% 6 Missing and 2 partials ⚠️
...untime/IO/SerializationTarget/BigSpanExtensions.cs 33.33% 6 Missing ⚠️
src/FlatSharp.Runtime/SerializationHelpers.cs 50.00% 5 Missing and 1 partial ⚠️
...rp.Runtime/IO/InputBuffer/InputBufferExtensions.cs 94.73% 3 Missing and 2 partials ⚠️
...untime/IO/InputBuffer/ReadOnlyMemoryInputBuffer.cs 80.95% 4 Missing ⚠️
...c/FlatSharp.Runtime/SortedVectorHelpersInternal.cs 96.22% 0 Missing and 2 partials ⚠️
...atSharp.Runtime/IO/InputBuffer/ArrayInputBuffer.cs 94.73% 1 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #468      +/-   ##
==========================================
- Coverage   96.06%   92.92%   -3.14%     
==========================================
  Files         124      126       +2     
  Lines        8888     9202     +314     
  Branches      740      797      +57     
==========================================
+ Hits         8538     8551      +13     
- Misses        260      521     +261     
- Partials       90      130      +40     
Files with missing lines Coverage Δ
src/FlatSharp.Runtime/BufferTooSmallException.cs 100.00% <ø> (ø)
...rc/FlatSharp.Runtime/GeneratedSerializerWrapper.cs 82.14% <100.00%> (+0.07%) ⬆️
src/FlatSharp.Runtime/IGeneratedSerializer.cs 100.00% <ø> (ø)
...tSharp.Runtime/IO/InputBuffer/MemoryInputBuffer.cs 100.00% <100.00%> (ø)
src/FlatSharp.Runtime/IO/ScalarSpanReader.cs 100.00% <ø> (ø)
src/FlatSharp.Runtime/IO/SharedStringWriter.cs 94.02% <100.00%> (ø)
src/FlatSharp.Runtime/SerializationContext.cs 92.04% <100.00%> (+2.30%) ⬆️
src/FlatSharp.Runtime/SortedVectorHelpers.cs 98.82% <100.00%> (ø)
src/FlatSharp.Runtime/VTables/VTable4.cs 92.30% <100.00%> (ø)
src/FlatSharp.Runtime/VTables/VTable8.cs 93.75% <100.00%> (ø)
... and 41 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eacf325...6a9e2be. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescourtney jamescourtney requested a review from Copilot April 28, 2025 04:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request updates the FlatSharp codebase to support 64‐bit sizes by changing various APIs from int to long and cleaning up obsolete polyfill usings for NETSTANDARD2_0. Key changes include:

  • Converting serialization methods’ return types (GetMaxSize, Write) and related exception properties from int to long.
  • Adjusting benchmark methods and constants to use long for lengths and serialization results.
  • Updating runtime configurations for benchmarks to target Core80.

Reviewed Changes

Copilot reviewed 109 out of 118 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/FlatSharp.Runtime/GlobalUsings.cs Removed obsolete polyfill global using for NETSTANDARD2_0.
src/FlatSharp.Runtime/GeneratedSerializerWrapper.cs Updated API signatures from int to long and adjusted parameter types for 64-bit support.
src/FlatSharp.Runtime/FSThrow.cs & BufferTooSmallException.cs Changed BufferTooSmall signature and SizeNeeded property to use long.
src/FlatSharp.Compiler/GlobalUsings.cs Removed obsolete polyfill global using for NETSTANDARD2_0.
Benchmarks/MicroBench.Current/SerializeBenchmarks.cs Updated benchmark methods to use long return types and introduced a new method for random entries serialization.
Benchmarks/MicroBench.Current/Program.cs Updated runtime configuration from Core70 to Core80 and adjusted job settings.
Benchmarks/MicroBench.Current/Constants.cs Added a new NestedTable random entries buffer and updated AllocateAndSerialize to use long.
Benchmarks/Benchmark/Serializers/FlatSharpValueStructs.cs & FlatSharpHelper.cs Updated helper methods for serialization to return and process long values instead of int.
Files not reviewed (9)
  • .github/workflows/MonoAotValidation.yml: Language not supported
  • src/Benchmarks/Benchmark/Benchmark.csproj: Language not supported
  • src/Benchmarks/Microbench.fbs: Language not supported
  • src/Directory.Build.props: Language not supported
  • src/Directory.Packages.props: Language not supported
  • src/FlatSharp.Compiler/FlatSharp.Compiler.csproj: Language not supported
  • src/FlatSharp.Compiler/FlatSharp.Compiler.nuspec: Language not supported
  • src/FlatSharp.Compiler/FlatSharp.Compiler.targets: Language not supported
  • src/FlatSharp.Runtime/FlatSharp.Runtime.csproj: Language not supported
Comments suppressed due to low confidence (1)

src/Benchmarks/MicroBench.Current/SerializeBenchmarks.cs:57

  • [nitpick] The method name 'Serialize_PrimitivesTable_RandomlyPopulated' suggests it serializes primitive tables but internally it uses the 'NestedTable.Serializer'. Consider renaming the method to better reflect the data type being serialized (e.g., Serialize_NestedTable_RandomEntries) for improved clarity.
public long Serialize_PrimitivesTable_RandomlyPopulated()


private static byte[] AllocateAndSerialize<T>(T value) where T : class, IFlatBufferSerializable<T>
{
byte[] buffer = new byte[value.Serializer.GetMaxSize(value)];
Copy link

Copilot AI Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the buffer allocated using an int-based length is sufficient to handle the long value returned by the Write method. A mismatch here might cause issues if the length exceeds int.MaxValue.

Suggested change
byte[] buffer = new byte[value.Serializer.GetMaxSize(value)];
long maxSize = value.Serializer.GetMaxSize(value);
if (maxSize > int.MaxValue)
{
throw new InvalidOperationException($"Serialized data size exceeds the maximum buffer size of {int.MaxValue} bytes.");
}
byte[] buffer = new byte[(int)maxSize];

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants