[wasm][debugger] Support new hot reload features#70097
Conversation
- Returning the number of the fields in the class including the ones added by hotreload.
Add a test with 2 enc adding classes in both of them. Adding in the dump changes typedef added.
|
Tagging subscribers to this area: @thaystg Issue DetailsSupporting new hotreload features on wasm debugger:
Also fixed when try to update a class adding a new static field and the class wasn't initialized yet.
|
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsSupporting new hotreload features on wasm debugger:
Also fixed when try to update a class adding a new static field and the class wasn't initialized yet.
|
lambdageek
left a comment
There was a problem hiding this comment.
Changes in hot_reload and debugger-agent lgtm. I don't understand the DebugStore changes yet, need to read a bit more of the debug proxy source.
@ilonatommy could you review, also
ilonatommy
left a comment
There was a problem hiding this comment.
Good job! I focused mostly on .cs sources.
radical
left a comment
There was a problem hiding this comment.
I'm new to the hot reload bits, but tried to review what I could.
| typeInfo = new TypeInfo(this, typeHandle, typeDefinition, asmMetadataReaderParm, logger); | ||
| TypesByName[typeInfo.FullName] = typeInfo; | ||
| TypesByToken[typeInfo.Token] = typeInfo; | ||
| } |
There was a problem hiding this comment.
This should continue after this.
| TypesByToken[typeInfo.Token] = typeInfo; | ||
| } | ||
| } | ||
| else |
There was a problem hiding this comment.
And else block should run only when operation == Default.
| if (methods.TryGetValue(asmMetadataReader.GetRowNumber(entry.Handle), out MethodInfo method)) | ||
| method.UpdateEnC(asmMetadataReaderParm, pdbMetadataReaderParm, methodIdx); | ||
| else if (typeInfo != null) |
There was a problem hiding this comment.
Would typeInfo == null here indicate a bug? should we log a message then?
| { | ||
| var methodDefinition = asmMetadataReaderParm.GetMethodDefinition(MetadataTokens.MethodDefinitionHandle(methodIdxAsm)); | ||
| int methodIdx = GetMethodDebugInformationIdx(pdbMetadataReaderParm, asmMetadataReader.GetRowNumber(entry.Handle)); | ||
| if (methods.TryGetValue(asmMetadataReader.GetRowNumber(entry.Handle), out MethodInfo method)) |
There was a problem hiding this comment.
nit: this is used in a few places here, so can be extracted to a local.
| if (source != null) | ||
| source.AddMethod(methodInfo); |
There was a problem hiding this comment.
source will never be null here, since GetOrAddSourceFile creates a new one if needed. Is there a case where it shouldn't do that?
Co-authored-by: Ankit Jain <radical@gmail.com>
Supporting new hotreload features on wasm debugger:
Also fixed when try to update a class adding a new static field and the class wasn't initialized yet.
Also fixed information passed to BrowserDebugProxy, we need metadata delta and pdb delta, we were only receiving pdb delta.