Add "cache add" functionality to project caching - Attempt 2#9214
Add "cache add" functionality to project caching - Attempt 2#9214rainersigwald merged 15 commits intodotnet:mainfrom
Conversation
…tnet#9188)" This reverts commit 3c910ba.
This reverts commit 180ff51.
This reverts commit 79ad288.
This reverts commit 70b4bcf.
| ManualResetEventSlim handle = _fileAccessCompletionWaitHandles.GetOrAdd(globalRequestId, static _ => new ManualResetEventSlim()); | ||
| handle.Set(); | ||
| } | ||
| else if (_tempDirectory != null && fileAccessPath.StartsWith(_tempDirectory)) |
There was a problem hiding this comment.
This StartsWith is culture specific while the previous one is ordinal.
| /// </summary> | ||
| /// <param name="fileAccessData">The file access to report.</param> | ||
| [CLSCompliant(false)] | ||
| public virtual void ReportFileAccess(FileAccessData fileAccessData) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Did you consider making this parameter in FileAccessData? This is a 9 member struct which means every call to this API is going to involve a lot of copies. This API is going to be called a lot in a tight loop for non-trivial build operations so it's one where efficiency may be important.
| Handlers[] localHandlers = _handlers; | ||
| foreach (Handlers handlers in localHandlers) | ||
| { | ||
| handlers.FileAccessHander.Invoke(buildRequest, fileAccessData); |
There was a problem hiding this comment.
This is another case where we're copying a large struct in a loop (this is nested in another loop so even more copies).
| /// <inheritdoc/> | ||
| public void Translate(ITranslator translator) => translator.Translate(ref _fileAccessData); | ||
|
|
||
| internal FileAccessData FileAccessData => _fileAccessData; |
There was a problem hiding this comment.
This is a place you should consider using
intenal ref readonly FileAccessData FileAccessData => ref _fileAccessData;Otherwise you're doing a full copy just to access one or two members.
| { | ||
| private FileAccessData _fileAccessData; | ||
|
|
||
| internal FileAccessReport(FileAccessData fileAccessData) => _fileAccessData = fileAccessData; |
There was a problem hiding this comment.
Another place to consider in FileAccessData
| /// Called for each file access from an MSBuild node or one of its children. | ||
| /// </summary> | ||
| [CLSCompliant(false)] | ||
| public virtual void HandleFileAccess(FileAccessContext fileAccessContext, FileAccessData fileAccessData) |
| /// Called for each new child process created by an MSBuild node or one of its children. | ||
| /// </summary> | ||
| [CLSCompliant(false)] | ||
| public virtual void HandleProcess(FileAccessContext fileAccessContext, ProcessData processData) |
| /// <inheritdoc/> | ||
| public NodePacketType Type => NodePacketType.ProcessReport; | ||
|
|
||
| internal ProcessData ProcessData => _processData; |
There was a problem hiding this comment.
A place where you can use ref readonly to alleviate copies
internal ref readonly ProcessData ProcessData => ref _processData;| { | ||
| private ProcessData _processData; | ||
|
|
||
| internal ProcessReport(ProcessData processData) => _processData = processData; |
| Handlers[] localHandlers = _handlers; | ||
| foreach (Handlers handlers in localHandlers) | ||
| { | ||
| handlers.ProcessHandler.Invoke(buildRequest, processData); |
There was a problem hiding this comment.
Copying a large struct in a loop
5 participants
This is a redo of #8726, but with fixes to make the changes acceptable for VS insertion.