Skip to content

Commit

Permalink
cache EECodeInfo based on given code pointer, not start of the method
Browse files Browse the repository at this point in the history
The EECodeInfo includes the relative offset (given ip - start of method) so it's not ok to share for different code pointers into the same method
  • Loading branch information
lambdageek committed Oct 9, 2024
1 parent f5995d6 commit eb190a2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
13 changes: 9 additions & 4 deletions docs/design/datacontracts/ExecutionManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ managed method corresponding to that address.
## APIs of contract

```csharp
internal struct EECodeInfoHandle
struct EECodeInfoHandle
{
// no public constructor
public readonly TargetPointer Address;
// no public constructor
internal EECodeInfoHandle(TargetPointer address) => Address = address;
}
```
Expand Down Expand Up @@ -75,13 +75,18 @@ The bulk of the work is donee by the `GetEECodeInfoHandle` API that maps a code
}
EECodeInfoHandle? IExecutionManager.GetEECodeInfoHandle(TargetCodePointer ip)
{
TargetPointer key = ip.AsTargetPointer;
if (/*cache*/.ContainsKey(key))
{
return new EECodeInfoHandle(key);
}
EECodeInfo? info = GetEECodeInfo(ip);
if (info == null || !info.Valid)
{
return null;
}
TargetPointer key = info.CodeHeaderAddress;
AddToCache(key, info);/* add a mapping from key to info to a chache */
/*cache*/.TryAdd(key, info);
return new EECodeInfoHandle(key);
return new EECodeInfoHandle(key);
}
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using Microsoft.Diagnostics.DataContractReader.ExecutionManagerHelpers;

namespace Microsoft.Diagnostics.DataContractReader.Contracts;

Expand All @@ -29,6 +28,8 @@ public ExecutionManager_1(Target target, Data.RangeSectionMap topRangeSectionMap
_r2rJitManager = new ReadyToRunJitManager(_target);
}

// Note, because of RelativeOffset, this code info is per code pointer, not per method
// TODO: rethink whether this makes sense. We don't need to copy the runtime's notion of EECodeInfo verbatim
private class EECodeInfo
{
private readonly int _codeHeaderOffset;
Expand Down Expand Up @@ -165,14 +166,16 @@ private JitManager GetJitManager(Data.RangeSection rangeSectionData)
}
EECodeInfoHandle? IExecutionManager.GetEECodeInfoHandle(TargetCodePointer ip)
{
// TODO: some kind of cache based on ip, too?
// we don't know if the ip is the start of the code.
TargetPointer key = ip.AsTargetPointer; // FIXME: thumb bit. It's harmless (we potentialy have 2 cache entries per IP), but we should fix it
if (_codeInfos.ContainsKey(key))
{
return new EECodeInfoHandle(key);
}
EECodeInfo? info = GetEECodeInfo(ip);
if (info == null || !info.Valid)
{
return null;
}
TargetPointer key = info.CodeHeaderAddress;
_codeInfos.TryAdd(key, info);
return new EECodeInfoHandle(key);
}
Expand Down

0 comments on commit eb190a2

Please sign in to comment.