Skip to content

[hot_reload] Add instance fields #76462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
lambdageek merged 55 commits into from
Nov 22, 2022
Merged

[hot_reload] Add instance fields #76462

lambdageek merged 55 commits into from
Nov 22, 2022

Conversation

@lambdageek
Copy link
Member

@lambdageek lambdageek commented Sep 30, 2022 •
edited
Loading

Fixes #63643

Remaining work:

  • get/set fields in the debugger
  • Add instance properties
  • verify ves_icall_property_info_get_default_valueworks with added props
  • Add instance events (need added skeletons)
  • mono_class_get_field_token
  • mono_class_get_event_token
  • mono_class_get_property_token
  • mono_field_get_rva
  • init_weak_fields_inner
  • ves_icall_RuntimeFieldInfo_GetFieldOffset
  • ves_icall_System_RuntimeFieldHandle_GetValueDirect/SetValueDirect
  • audit callers of mono_handle_unsafe_field_addr
  • mono_metadata_field_info_full
  • mono_metadata_get_marshal_info, possibly

@ghost ghost assigned lambdageek Sep 30, 2022
@ghost ghost added the area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc label Sep 30, 2022
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from 0a73fe5 to 0da7ebf Compare October 4, 2022 20:15
@lambdageek lambdageek changed the title [mbr] Add instance fields [hot_reload] Add instance fields Oct 4, 2022
@lambdageek lambdageek marked this pull request as ready for review October 4, 2022 20:24
@lambdageek
Copy link
Member Author

lambdageek commented Oct 4, 2022

@thaystg @BrzVlad could you take a look. Vlad: I'm adding an interp opcode and some transform changes to support it

@lambdageek
Copy link
Member Author

lambdageek commented Oct 4, 2022

(Some of the code is part of #76618 )

@lambdageek
Copy link
Member Author

lambdageek commented Oct 4, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Oct 4, 2022

Azure Pipelines successfully started running 1 pipeline(s).

BrzVlad
BrzVlad reviewed Oct 5, 2022
View reviewed changes
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from ca44d25 to c0fba82 Compare October 5, 2022 19:17
@lambdageek
Copy link
Member Author

lambdageek commented Oct 5, 2022

Found a few more items that need work before this is ready; updated the PR description

@lambdageek
Copy link
Member Author

lambdageek commented Oct 5, 2022

Interesting. CoreCLR is crashing on something that I'm doing in the AddInstanceField test that I added in this PR 😁

@lambdageek lambdageek mentioned this pull request Oct 6, 2022
Closed
@lambdageek
Copy link
Member Author

lambdageek commented Oct 6, 2022

CoreCLR failure is #76702

@build-analysis build-analysis bot mentioned this pull request Oct 6, 2022
Closed
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from 17a3e9e to c933746 Compare October 25, 2022 19:58
@lambdageek lambdageek requested a review from radical as a code owner October 25, 2022 19:58
@build-analysis build-analysis bot mentioned this pull request Oct 25, 2022
Closed
2 tasks
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from c933746 to 616033b Compare November 10, 2022 21:08
This was referenced Nov 11, 2022
Closed
Open
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from 139dfd1 to 517978e Compare November 15, 2022 23:47
lambdageek and others added 8 commits November 17, 2022 09:46
@lambdageek
WIP checkpoint add_props
619d956 
Note: we're not doing anything with the new PropertyMap row

Note2: we get MethodSemantics rows for properties that got updated,
and we're currently ignoring them.  Need to check that this is
reasonable. (A test would be to use reflection to grab a getter or
setter whose method body was changed and then try and invoke it and
verify that we're calling the correct method.)
@lambdageek
fixup comment - repeated MethodSemantics rows can happen
9b6503a
@lambdageek
added properties iteration
21dad8b
@lambdageek
protect callers of mono_class_get_property_token
2bc5e5a
@lambdageek
fix build warnings
ac34462
@lambdageek
fix dynamic component builds
fbcd1b2
@lambdageek
make mono_class_get_property_token work for added props
2934993
@lambdageek
add test for adding instance event
9c4c18a
@lambdageek lambdageek force-pushed the hot-reload-add-innstance branch from 78dedf7 to 2934993 Compare November 18, 2022 19:22
@lambdageek
Copy link
Member Author

lambdageek commented Nov 21, 2022 •
edited
Loading

@thaystg @fanyang-mono This PR is ready. Could you please take another look.

thaystg
thaystg reviewed Nov 21, 2022
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request Nov 21, 2022
Closed
@lambdageek lambdageek merged commit 2ae7f5d into dotnet:main Nov 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@BrzVlad BrzVlad BrzVlad left review comments

@thaystg thaystg thaystg approved these changes

@vargaz vargaz Awaiting requested review from vargaz

@marek-safar marek-safar Awaiting requested review from marek-safar

@radical radical Awaiting requested review from radical

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

Assignees

@lambdageek lambdageek

Labels

area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[mono] Hot Reload: support adding instance fields

3 participants

@lambdageek @thaystg @BrzVlad