KalleOlaviNiemitalo commented on code in PR #2439:
URL: https://github.com/apache/avro/pull/2439#discussion_r1293381830
##########
lang/csharp/src/apache/test/File/FileTests.cs:
##########
@@ -668,6 +668,55 @@ public void TestPartialReadAll([Values(specificSchema)]
string schemaStr, [Value
}
}
+ [Test]
+ public void TestDeflateReadMemoryUsage([Values(specificSchema)] string
schemaStr)
+ {
+ // create and write out
+ IList<Foo> records = MakeRecords(GetTestFooObject());
+
+ Process currentProcess = Process.GetCurrentProcess();
+
+ MemoryStream dataFileOutputStream = new MemoryStream();
+
+ Schema schema = Schema.Parse(schemaStr);
+ DatumWriter<Foo> writer = new SpecificWriter<Foo>(schema);
+ using (IFileWriter<Foo> dataFileWriter =
DataFileWriter<Foo>.OpenWriter(writer, dataFileOutputStream,
Codec.CreateCodec(Codec.Type.Deflate)))
+ {
+ for (int i = 0; i < 10; ++i)
+ {
+ foreach (Foo foo in records)
+ {
+ dataFileWriter.Append(foo);
+ }
+
+ // write out block
+ if (i == 1 || i == 4)
+ {
+ dataFileWriter.Sync();
+ }
+ }
+ }
+
+ long startMemoryUsedBytes = currentProcess.WorkingSet64;
+
+ MemoryStream dataFileInputStream = new
MemoryStream(dataFileOutputStream.ToArray());
+ dataFileInputStream.Position = 0;
+
+ // read back
+ IList<Foo> readRecords = new List<Foo>();
+ using (IFileReader<Foo> reader =
DataFileReader<Foo>.OpenReader(dataFileInputStream, schema))
+ {
+ // read records from synced position
+ foreach (Foo rec in reader.NextEntries)
+ readRecords.Add(rec);
+ }
+
+ long totalMemoryUsedBytes = currentProcess.WorkingSet64 -
startMemoryUsedBytes;
+
+ Assert.IsTrue(totalMemoryUsedBytes == 0, "Total memory usage in
working set");
Review Comment:
This test looks unreliable:
* WorkingSet64 is the amount of physical memory reserved for the process.
If the process allocates some memory and doesn't use it afterwards, and the
operating system pages it out, then it won't be included in WorkingSet64.
* Process.WorkingSet64 updates only when you call Process.Refresh().
* Can other tests be run in parallel with this one? If they can, the memory
allocated by them could cause this test to fail.
* The test code between the `currentProcess.WorkingSet64` evaluations
allocates new objects whose memory might not be garbage-collected and released
to the OS soon enough. The runtime can also allocate memory for JIT-compiled
methods.
The test might become more reliable if it were changed to:
* Run the decompression a few times before the measurement, to get to a
stable state.
* Measure AppDomain.MonitoringTotalAllocatedMemorySize or
GC.GetAllocatedBytesForCurrentThread(), rather than Process.WorkingSet64.
* Allow some amount of allocations but not too much. This might require a
larger data file, in order to distinguish the important allocations from noise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
