Fix Merge operation failed for EtwProfiler #1545
Conversation
…very general Assert.True
adamsitnik
changed the title
|
@AndreyAkinshin PTAL |
There was a problem hiding this comment.
@adamsitnik IMO you fixed the problem but I have some minor comments.
| // long paths can be enabled on Windows but it does not mean that ETW is going to work fine.. | ||
| // so we always use 260 as limit on Windows | ||
| int limit = RuntimeInformation.IsWindows() | ||
| ? WindowsOldPathLimit - "userheap.etl".Length // the session files get merged, they need to have same name (without extension) |
There was a problem hiding this comment.
You could reuse HeapSession.FileExtension. I checked it seems to be the longest extension, so it is a good solution.
There was a problem hiding this comment.
You could reuse HeapSession.FileExtension
I was thinking about it. The problem is that it's an instance property - so I would need to have an HeapSession instance to get it.
Then I was thinking about introducing an internal const string field to the HeapSession but it was far from perfect:
- I would need two names for the same thing
- it would be still possible to introduce a new session type with a longer name.
I think that we can keep it as it is until we find a better solution.
| { | ||
| [GenericTypeArguments(typeof(byte))] // value type | ||
| [GenericTypeArguments(typeof(object))] // reference type | ||
| public class RentReturnArrayPoolTests<T> |
There was a problem hiding this comment.
Why do you need such a complicated example?
There was a problem hiding this comment.
@WojciechNagorski to get a benchmark with a very long name. I was lazy and I just copied it 1:1 from the bug report
Update error message to reflect the change introduced by dotnet#1545
Update error message to reflect the change introduced by #1545
Fixes #1544
to tell the long story short, if Long Paths support is enabled on Windows, the ETW might still throw errors.
The fix is to always use 260 characters limit