From b6f9c90d6380dd6033990c21498a54c6a9261175 Mon Sep 17 00:00:00 2001 From: Oleksii Kononenko Date: Mon, 31 Jul 2023 23:56:04 +0000 Subject: [PATCH] Merged PR 730784: Revert "Merged PR 727953: More robust execution-log event serialization Revert "Merged PR 727953: More robust execution-log event serialization This PR makes serialization of execution-log event serialization more robust by handling the exception, e.g., OutOfMemory exception, that can happen during serialization. In the case of an exception, the event is skipped, and users are given a warning about this. Related work items: #2081944" Reverted commit `e2ca19cb`. Related work items: #2081944 --- .../Scheduler/Tracing/ExecutionLog.Events.cs | 79 ++------ .../Engine/Scheduler/Tracing/ExecutionLog.cs | 28 +-- Public/Src/Engine/Scheduler/Tracing/Log.cs | 33 ++-- .../Engine/Scheduler/Tracing/LogEventId.cs | 1 - .../UnitTests/Scheduler/ExecutionLogTests.cs | 177 ++---------------- .../Utilities/Tracing/BinaryLogger.cs | 129 ++++--------- 6 files changed, 92 insertions(+), 355 deletions(-) diff --git a/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.Events.cs b/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.Events.cs index e9737ef1c2..c62bcd5c0b 100644 --- a/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.Events.cs +++ b/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.Events.cs @@ -129,11 +129,6 @@ public interface IExecutionLogTarget : IDisposable /// Build Manifest hash and relative file path is reported /// void RecordFileForBuildManifest(RecordFileForBuildManifestEventData data); - - /// - /// Test custom event. Used only by unit tests. - /// - void TestCustom(TestCustomEventData data); } /// @@ -220,11 +215,6 @@ public enum ExecutionEventId : byte /// See /// DynamicDirectoryContentsDecided = 16, - - /// - /// See - /// - TestCustom = 17, } /// @@ -354,36 +344,27 @@ public static class ExecutionLogMetadata ExecutionEventId.RecordFileForBuildManifest, (data, target) => target.RecordFileForBuildManifest(data)); - /// - /// Event description for - /// - public static readonly ExecutionLogEventMetadata TestCustom = - new ExecutionLogEventMetadata( - ExecutionEventId.TestCustom, - (data, target) => target.TestCustom(data)); - /// /// All execution log events. /// public static readonly IReadOnlyList Events = new ExecutionLogEventMetadata[] - { - BxlInvocation, - FileArtifactContentDecided, - WorkerList, - PipExecutionPerformance, - DirectoryMembershipHashed, - ProcessExecutionMonitoringReported, - BuildSessionConfiguration, - DependencyViolationReported, - PipExecutionStepPerformanceReported, - ProcessFingerprintComputation, - PipCacheMiss, - PipExecutionDirectoryOutputs, - CacheMaterializationError, - RecordFileForBuildManifest, - DynamicDirectoryContentsDecided, - TestCustom, - }; + { + BxlInvocation, + FileArtifactContentDecided, + WorkerList, + PipExecutionPerformance, + DirectoryMembershipHashed, + ProcessExecutionMonitoringReported, + BuildSessionConfiguration, + DependencyViolationReported, + PipExecutionStepPerformanceReported, + ProcessFingerprintComputation, + PipCacheMiss, + PipExecutionDirectoryOutputs, + CacheMaterializationError, + RecordFileForBuildManifest, + DynamicDirectoryContentsDecided + }; } /// @@ -1482,30 +1463,4 @@ public void DeserializeAndUpdate(BinaryLogReader.EventReader reader) r => (reader.ReadFileArtifact(), new ContentHash(reader))); } } - - /// - /// Custom event for testing purposes. - /// - [SuppressMessage("Microsoft.Performance", "CA1815:OverrideEqualsAndOperatorEqualsOnValueTypes")] - public struct TestCustomEventData : IExecutionLogEventData - { - /// - /// Custom serialization function. - /// - public static Action SerializeFunc; - - /// - /// Custom deserialization function. - /// - public static Action DeserializeAndUpdateFunc; - - /// - public ExecutionLogEventMetadata Metadata => ExecutionLogMetadata.TestCustom; - - /// - public void DeserializeAndUpdate(BinaryLogReader.EventReader reader) => DeserializeAndUpdateFunc(reader); - - /// - public void Serialize(BinaryLogger.EventWriter writer) => SerializeFunc(writer); - } } diff --git a/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.cs b/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.cs index dc81b2a80f..ad8c530e54 100644 --- a/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.cs +++ b/Public/Src/Engine/Scheduler/Tracing/ExecutionLog.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics.ContractsLight; using System.IO; +using BuildXL.Utilities; using BuildXL.Utilities.Core; using BuildXL.Utilities.Tracing; using static BuildXL.Utilities.Core.FormattableStringEx; @@ -241,12 +242,6 @@ public virtual void DynamicDirectoryContentsDecided(DynamicDirectoryContentsDeci { ReportUnhandledEvent(data); } - - /// - public virtual void TestCustom(TestCustomEventData data) - { - ReportUnhandledEvent(data); - } } /// @@ -266,7 +261,10 @@ public abstract class ExecutionLogEventMetadata public readonly ExecutionEventId EventId; /// - protected ExecutionLogEventMetadata(ExecutionEventId eventId) => EventId = eventId; + protected ExecutionLogEventMetadata(ExecutionEventId eventId) + { + EventId = eventId; + } /// /// Translates a serialized stream containing an event (of this type) into an invocation on an . @@ -295,7 +293,7 @@ public ExecutionLogEventMetadata(ExecutionEventId eventId, Action public override void DeserializeAndLogToTarget(BinaryLogReader.EventReader eventReader, IExecutionLogTarget target) { - TEventData data = default; + TEventData data = default(TEventData); data.DeserializeAndUpdate(eventReader); m_process(data, target); } @@ -387,19 +385,7 @@ private void Log(TEventData data) where TEventData : struct, IExecut { using (BinaryLogger.EventScope eventScope = m_logFile.StartEvent((uint)data.Metadata.EventId, m_workerId)) { - - try - { - data.Serialize(eventScope.Writer); - } - catch (Exception e) - { - // Set exception so that event is not serialized to the log file. - eventScope.SetException(e); - - // Inform users that an event is not logged due to an exception. - Logger.Log.FailedLoggingExecutionLogEventData(Events.StaticContext, data.Metadata.EventId.ToString(), e.ToString()); - } + data.Serialize(eventScope.Writer); } } diff --git a/Public/Src/Engine/Scheduler/Tracing/Log.cs b/Public/Src/Engine/Scheduler/Tracing/Log.cs index 046b414f46..94fc0ae286 100644 --- a/Public/Src/Engine/Scheduler/Tracing/Log.cs +++ b/Public/Src/Engine/Scheduler/Tracing/Log.cs @@ -3975,31 +3975,22 @@ internal abstract void ProcessPipExecutionInfo( internal abstract void UnableToMonitorDriveWithSubst(LoggingContext loggingContext, string path, string drive); [GeneratedEvent( - (ushort)LogEventId.SchedulerCompleteExceptMaterializeOutputs, - EventGenerators = EventGenerators.LocalOnly, - EventLevel = Level.Verbose, - Keywords = (int)Keywords.UserMessage, - EventTask = (ushort)Tasks.Scheduler, - Message = "The scheduler has been marked completed except for MaterializeOutput pip steps")] + (ushort)LogEventId.SchedulerCompleteExceptMaterializeOutputs, + EventGenerators = EventGenerators.LocalOnly, + EventLevel = Level.Verbose, + Keywords = (int)Keywords.UserMessage, + EventTask = (ushort)Tasks.Scheduler, + Message = "The scheduler has been marked completed except for MaterializeOutput pip steps")] internal abstract void SchedulerCompleteExceptMaterializeOutputs(LoggingContext loggingContext); [GeneratedEvent( - (ushort)LogEventId.CreationTimeNotSupported, - EventGenerators = EventGenerators.LocalOnly, - EventLevel = Level.Verbose, - Keywords = (int)Keywords.UserMessage, - EventTask = (ushort)Tasks.Scheduler, - Message = "File creation time retrieval is not supported by the underlying operating system. Some optimizations will be disabled.")] + (ushort)LogEventId.CreationTimeNotSupported, + EventGenerators = EventGenerators.LocalOnly, + EventLevel = Level.Verbose, + Keywords = (int)Keywords.UserMessage, + EventTask = (ushort)Tasks.Scheduler, + Message = "File creation time retrieval is not supported by the underlying operating system. Some optimizations will be disabled.")] internal abstract void CreationTimeNotSupported(LoggingContext loggingContext); - - [GeneratedEvent( - (ushort)LogEventId.FailedLoggingExecutionLogEventData, - EventGenerators = EventGenerators.LocalOnly, - EventLevel = Level.Warning, - Keywords = (int)Keywords.UserMessage, - EventTask = (ushort)Tasks.Scheduler, - Message = "Failed logging execution log event data '{eventId}'. This does not impact build correctness but will cause the execution log to be incomplete for post-build analysis. Failure reason: {error}")] - internal abstract void FailedLoggingExecutionLogEventData(LoggingContext loggingContext, string eventId, string error); } } #pragma warning restore CA1823 // Unused field diff --git a/Public/Src/Engine/Scheduler/Tracing/LogEventId.cs b/Public/Src/Engine/Scheduler/Tracing/LogEventId.cs index 73945f17d0..87425b7fbe 100644 --- a/Public/Src/Engine/Scheduler/Tracing/LogEventId.cs +++ b/Public/Src/Engine/Scheduler/Tracing/LogEventId.cs @@ -520,7 +520,6 @@ public enum LogEventId : ushort FailedToDeserializeLRUMap = 14536, CreationTimeNotSupported = 14537, - FailedLoggingExecutionLogEventData = 14538, // was DependencyViolationGenericWithRelatedPip_AsError = 25000, // was DependencyViolationGeneric_AsError = 25001, diff --git a/Public/Src/Engine/UnitTests/Scheduler/ExecutionLogTests.cs b/Public/Src/Engine/UnitTests/Scheduler/ExecutionLogTests.cs index c8593ec598..983e650958 100644 --- a/Public/Src/Engine/UnitTests/Scheduler/ExecutionLogTests.cs +++ b/Public/Src/Engine/UnitTests/Scheduler/ExecutionLogTests.cs @@ -38,7 +38,7 @@ public ExecutionLogTests(ITestOutputHelper output) [Fact] public void TestRoundTripExecutionLog() { - TestExecutionLogHelper(ExpectProcessMonitoringEventData); + TestExecutionLogHelper(verifier => ExpectProcessMonitoringEventData(verifier)); } [Fact] @@ -78,155 +78,22 @@ public void TestPipCacheMissEventData() public void TestPipExecutionDirectoryOutputs() { TestExecutionLogHelper(verifier => - { - verifier.Expect(new PipExecutionDirectoryOutputs - { - PipId = new PipId(123), - DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( - ( - CreateDirectory(), - ReadOnlyArray.FromWithoutCopy(CreateSourceFile(), CreateOutputFile()) - ), - ( - CreateDirectory(), - ReadOnlyArray.FromWithoutCopy(CreateOutputFile(), CreateSourceFile()) - ) - ) - }); - }); - } - - [Fact] - public void TestCustomEvent() - { - TestExecutionLogHelper(verifier => - { - var pipId = new PipId(123); - var sourceFile = CreateSourceFile(); - var outputFile = CreateOutputFile(); - - TestCustomEventData.SerializeFunc = writer => - { - pipId.Serialize(writer); - writer.Write(sourceFile); - writer.Write(outputFile); - }; - - TestCustomEventData.DeserializeAndUpdateFunc = reader => - { - var dPipId = PipId.Deserialize(reader); - var dSourceFile = reader.ReadFileArtifact(); - var dOutputFile = reader.ReadFileArtifact(); - XAssert.AreEqual(pipId, dPipId); - XAssert.AreEqual(sourceFile, dSourceFile); - XAssert.AreEqual(outputFile, dOutputFile); - }; - - verifier.Expect(new TestCustomEventData()); - }); - } - - [Fact] - public void TestSkipEventDueToFailedSerialization() - { - TestExecutionLogHelper(verifier => - { - var outputFile1 = CreateOutputFile(); - var outputFile2 = CreateOutputFile(); - var outputFile3 = CreateOutputFile(); - var outputFile4 = CreateOutputFile(); - - verifier.Expect(new PipExecutionDirectoryOutputs - { - PipId = new PipId(123), - DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( - ( - CreateDirectory(), - ReadOnlyArray.FromWithoutCopy(outputFile1, outputFile2) - )) - }); - - TestCustomEventData.SerializeFunc = writer => - { - writer.Write(outputFile2); - writer.Write(outputFile3); - throw new OutOfMemoryException("Fake OOM exception"); - }; - - TestCustomEventData.DeserializeAndUpdateFunc = reader => - { - XAssert.Fail("Deserialization should never be called due to failed serialization"); - }; - - verifier.LogUnexpectedExpect(new TestCustomEventData()); - - verifier.Expect(new PipExecutionDirectoryOutputs - { - PipId = new PipId(234), - DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( - ( - CreateDirectory(), - // Although outputFile2 was not serialized by the test custom event, it should still be - // serialized by this event, and so the deserialization should succeed. - ReadOnlyArray.FromWithoutCopy(outputFile2, outputFile3, outputFile4) - )) - }); - }); - } - - [Fact] - [Trait("Category", "LongRunningTest")] - public void TestSkipEventDueToTooLargeSerialization() - { - TestExecutionLogHelper(verifier => - { - var outputFile1 = CreateOutputFile(); - var outputFile2 = CreateOutputFile(); - var outputFile3 = CreateOutputFile(); - var outputFile4 = CreateOutputFile(); - var dummyFile = CreateOutputFile(); - - verifier.Expect(new PipExecutionDirectoryOutputs - { - PipId = new PipId(123), - DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( - ( - CreateDirectory(), - ReadOnlyArray.FromWithoutCopy(outputFile1, outputFile2) - )) - }); - - TestCustomEventData.SerializeFunc = writer => - { - writer.Write(outputFile2); - writer.Write(outputFile3); - - // The following should cause OutOfMemory exception on the underlying MemoryStream. - while (true) - { - writer.Write(dummyFile); - } - }; - - TestCustomEventData.DeserializeAndUpdateFunc = reader => - { - XAssert.Fail("Deserialization should never be called due to failed serialization"); - }; - - verifier.LogUnexpectedExpect(new TestCustomEventData()); - - verifier.Expect(new PipExecutionDirectoryOutputs - { - PipId = new PipId(234), - DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( - ( - CreateDirectory(), - // Although outputFile2 was not serialized by the test custom event, it should still be - // serialized by this event, and so the deserialization should succeed. - ReadOnlyArray.FromWithoutCopy(outputFile2, outputFile3, outputFile4) - )) - }); - }); + { + verifier.Expect(new PipExecutionDirectoryOutputs + { + PipId = new PipId(123), + DirectoryOutputs = ReadOnlyArray<(DirectoryArtifact, ReadOnlyArray)>.FromWithoutCopy( + ( + CreateDirectory(), + ReadOnlyArray.FromWithoutCopy(CreateSourceFile(), CreateOutputFile()) + ), + ( + CreateDirectory(), + ReadOnlyArray.FromWithoutCopy(CreateOutputFile(), CreateSourceFile()) + ) + ) + }); + }); } [Fact] @@ -447,8 +314,7 @@ private class ExecutionLogVerifier : ExecutionLogTargetBase, IEqualityVerifier, IEqualityVerifier, IEqualityVerifier, - IEqualityVerifier, - IEqualityVerifier + IEqualityVerifier { private readonly Queue m_expectedData = new Queue(); private readonly ExecutionLogTests m_parent; @@ -493,11 +359,6 @@ public override void PipExecutionDirectoryOutputs(PipExecutionDirectoryOutputs d VerifyEvent(data); } - public override void TestCustom(TestCustomEventData data) - { - VerifyEvent(data); - } - public virtual void Expect(TEventData data) where TEventData : struct, IExecutionLogEventData { @@ -659,8 +520,6 @@ public bool VerifyEquals(PipExecutionDirectoryOutputs expected, PipExecutionDire return true; } - public bool VerifyEquals(TestCustomEventData expected, TestCustomEventData actual) => true; - private class DirectoryOutputComparer : IEqualityComparer<(DirectoryArtifact directoryArtifact, ReadOnlyArray contents)> { public static readonly DirectoryOutputComparer Instance = new DirectoryOutputComparer(); diff --git a/Public/Src/Utilities/Utilities/Tracing/BinaryLogger.cs b/Public/Src/Utilities/Utilities/Tracing/BinaryLogger.cs index e8afdae09a..71926a3cfe 100644 --- a/Public/Src/Utilities/Utilities/Tracing/BinaryLogger.cs +++ b/Public/Src/Utilities/Utilities/Tracing/BinaryLogger.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.ContractsLight; using System.IO; @@ -49,8 +48,8 @@ public sealed class BinaryLogger : IDisposable // Pending events queue and corresponding draining thread. // The queue could be more easily modeled with a BlockingCollection (which doesn't need any explicit synchronization primitives), but noticeable thread contention // was found under high load of events being added. - private readonly ConcurrentQueue m_pendingEvents = new(); - private readonly AutoResetEvent m_eventsAvailable = new(false); + private readonly ConcurrentQueue m_pendingEvents = new ConcurrentQueue(); + private readonly AutoResetEvent m_eventsAvailable = new AutoResetEvent(false); private readonly Thread m_pendingEventsDrainingThread; private bool m_completeAdding = false; @@ -80,13 +79,11 @@ public BinaryLogger(Stream logStream, PipExecutionContext context, Guid logId, i m_logStreamWriter = new BuildXLWriter(debug: false, stream: logStream, leaveOpen: !closeStreamOnDispose, logStats: false); m_watch = Stopwatch.StartNew(); m_capturedPaths = new ConcurrentBigMap(); - m_capturedStrings = new ConcurrentBigMap - { - { StringId.Invalid, true } - }; + m_capturedStrings = new ConcurrentBigMap(); + m_capturedStrings.Add(StringId.Invalid, true); m_writerPool = new ObjectPool( () => new EventWriter(this), - writer => { writer.Clear(); return writer; }); + writer => { writer.Seek(0, SeekOrigin.Begin); return writer; }); m_onEventWritten = onEventWritten; var logIdBytes = logId.ToByteArray(); @@ -141,8 +138,6 @@ private void WriteEventDataAndReturnWriter(EventWriter eventWriter) m_logStreamWriter.WriteCompact((int)eventPayloadStream.Position); m_logStreamWriter.Write(eventPayloadStream.GetBuffer(), 0, (int)eventPayloadStream.Position); - eventWriter.UpdateCapturedPathsAndStrings(); - m_writerPool.PutInstance(eventWriter); m_onEventWritten?.Invoke(); } @@ -167,9 +162,15 @@ public Task FlushAsync() /// the event id /// the worker id /// scope containing writer for event payload - public EventScope StartEvent(uint eventId, uint workerId) => new(GetEventWriterWrapper(checked(eventId + (uint)LogSupportEventId.Max), workerId: workerId)); + public EventScope StartEvent(uint eventId, uint workerId) + { + return new EventScope(GetEventWriterWrapper(checked(eventId + (uint)LogSupportEventId.Max), workerId: workerId)); + } - private EventScope StartEvent(LogSupportEventId eventId) => new(GetEventWriterWrapper((uint)eventId, workerId: 0)); + private EventScope StartEvent(LogSupportEventId eventId) + { + return new EventScope(GetEventWriterWrapper((uint)eventId, workerId: 0)); + } private EventWriter GetEventWriterWrapper(uint eventId, uint workerId) { @@ -207,8 +208,10 @@ private void QueueAction(IQueuedAction action) /// the date private void LogStartTime(DateTime startTime) { - using var eventScope = StartEvent(LogSupportEventId.StartTime); - eventScope.Writer.Write(startTime.ToFileTimeUtc()); + using (var eventScope = StartEvent(LogSupportEventId.StartTime)) + { + eventScope.Writer.Write(startTime.ToFileTimeUtc()); + } } /// @@ -287,7 +290,7 @@ internal enum AbsolutePathType : byte /// Scope for writing data for an event /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1815:OverrideEqualsAndOperatorEqualsOnValueTypes")] - public readonly struct EventScope : IDisposable + public struct EventScope : IDisposable { /// /// The writer for writing event data @@ -297,28 +300,18 @@ internal enum AbsolutePathType : byte /// /// Class constructor. INTERNAL USE ONLY. /// - internal EventScope(EventWriter writer) => Writer = writer; + internal EventScope(EventWriter writer) + { + Writer = writer; + } /// /// Writes the event data /// public void Dispose() { - // Skip event when there is an exception. - // This also avoids returning a broken Writer to the pool. - if (Writer.Exception == null) - { - Writer.Logger.QueueAction(Writer); - } + Writer.Logger.QueueAction(Writer); } - - /// - /// Sets the exception that happened during writing event data with this scope. - /// - /// - /// When the exception is set, the event is not logged. - /// - public void SetException(Exception exception) => Writer.Exception = exception; } private interface IQueuedAction : IDisposable @@ -331,7 +324,10 @@ private class FlushAction : IQueuedAction private readonly TaskSourceSlim m_taskSource = TaskSourceSlim.Create(); private readonly BinaryLogger m_binaryLogger; - public FlushAction(BinaryLogger binaryLogger) => m_binaryLogger = binaryLogger; + public FlushAction(BinaryLogger binaryLogger) + { + m_binaryLogger = binaryLogger; + } public Task Completion => m_taskSource.Task; @@ -360,24 +356,23 @@ public sealed class EventWriter : BuildXLWriter, IQueuedAction internal long Timestamp; internal uint WorkerId; private readonly BinaryLogger m_logWriter; - private readonly HashSet m_eventCapturedPaths = new(); - private readonly HashSet m_eventCapturedStrings = new(); /// /// Returns the underlying log logger. Should be used only inside EventScope.Dispose() /// - internal BinaryLogger Logger => m_logWriter; + internal BinaryLogger Logger { get { return m_logWriter; } } internal PathTable PathTable => m_logWriter.m_context.PathTable; - internal Exception Exception { get; set; } - /// /// Class constructor /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] internal EventWriter(BinaryLogger logWriter) - : base(debug: false, stream: new MemoryStream(), leaveOpen: false, logStats: false) => m_logWriter = logWriter; + : base(debug: false, stream: new MemoryStream(), leaveOpen: false, logStats: false) + { + m_logWriter = logWriter; + } /// /// Writes an absolute path. @@ -401,8 +396,7 @@ public override void Write(AbsolutePath value) { Write((byte)AbsolutePathType.Dynamic); var result = m_logWriter.m_capturedPaths.GetOrAdd(value, false); - var eventResult = m_eventCapturedPaths.Contains(value); - if (!result.Item.Value && !eventResult) + if (!result.Item.Value) { using (var eventScope = m_logWriter.StartEvent(LogSupportEventId.AddPath)) { @@ -414,25 +408,7 @@ public override void Write(AbsolutePath value) : value.ToString(m_logWriter.m_context.PathTable)); } - // We cannot yet track the captured path in m_logWriter because writing the event using this - // EventWriter can fail before the buffered memory stream is flushed to the stream of m_logWriter in - // BinaryLogger.WriteEventDataAndReturnWriter. Thus, we capture the path locally in this EventWriter - // in m_eventCapturedPath. The update to the m_logWriter.m_capturedPaths is done in UpdateCapturedPathsAndStrings - // which gets called in BinaryLogger.WriteEventDataAndReturnWriter. - // - // Note that we still need to "GetOrAdd" the path to m_logWriter.m_capturedPaths because we need to reserve a slot/index. - // - // If we don't do this, consider the following scenario: - // - The first event containing a path p is written using an EventWriter. - // - The path p is added to m_logWriter.m_capturedPaths. - // - Before the event is flushed to the stream of m_logWriter, an exception, e.g., OutOfMemory exception, is thrown, - // and the event is not written to the stream of m_logWriter. In particular, the encoding of the path p (the parent - // and file name) is not persisted in the stream of m_logWriter. - // - The second event containing the same path p is written using another EventWriter. - // - On writing p, this method sees that p has been captured by m_logWriter.m_capturedPaths and thus it only - // serializes the index. However, since this serialization doesn't include the encoding of the parent and file name - // of path p, the deserialization cannot recover the path only from the serialized index. - m_eventCapturedPaths.Add(value); + m_logWriter.m_capturedPaths[value] = true; } WriteCompact(result.Index); @@ -445,8 +421,7 @@ public override void Write(AbsolutePath value) public void WriteDynamicStringId(StringId value) { var result = m_logWriter.m_capturedStrings.GetOrAdd(value, false); - var eventResult = m_eventCapturedStrings.Contains(value); - if (!result.Item.Value && !eventResult) + if (!result.Item.Value) { using (var eventScope = m_logWriter.StartEvent(LogSupportEventId.AddStringId)) { @@ -454,44 +429,16 @@ public void WriteDynamicStringId(StringId value) eventScope.Writer.Write(value.ToString(m_logWriter.m_context.StringTable)); } - // See the comment in Write(AbsolutePath) for why we need to capture the string locally in this EventWriter. - m_eventCapturedStrings.Add(value); + m_logWriter.m_capturedStrings[value] = true; } WriteCompact(result.Index); } - void IQueuedAction.Run() => m_logWriter.WriteEventDataAndReturnWriter(this); - - /// - /// Clears this instance of . - /// - internal void Clear() + void IQueuedAction.Run() { - Seek(0, SeekOrigin.Begin); - m_eventCapturedPaths.Clear(); - m_eventCapturedStrings.Clear(); + m_logWriter.WriteEventDataAndReturnWriter(this); } - - /// - /// Copies the captured paths and strings from processing this event writer to the global maps maintained by the logger. - /// - /// - /// This method should be called only after the event data has been transferred to the log writer. - /// - internal void UpdateCapturedPathsAndStrings() - { - foreach (var path in m_eventCapturedPaths) - { - m_logWriter.m_capturedPaths.AddOrUpdate(path, false, (p, d) => true, (p, d, b) => true); - } - - foreach (var str in m_eventCapturedStrings) - { - m_logWriter.m_capturedStrings.AddOrUpdate(str, false, (s, d) => true, (s, d, b) => true); - } - } - } } }