[mono] Migrate more ghashtables to simdhash #102476
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 21, 2024
kg
requested review from
BrzVlad,
kotlarmilos,
lambdageek,
steveisok,
thaystg and
marek-safar
as code owners
lambdageek
reviewed
May 29, 2024
(this message is wrong due to rebasing) Use dn_simdhash_string_ptr for namespace lookup cache Use simdhash for interp patch_sites_table Migrate method and methodref cache to simdhash Add missing include, fix typo Migrate data_hash and patchsite_hash to simdhash Pre-reserve capacity for some simdhash instances that grow during startup Reduce initial size of these simdhashes because we have one instance per assembly Fix bundled_resources by adding ANOTHER hash table Migrate MonoJitMemoryManager::seq_points to simdhash Add equivalent operation for g_hash_table_insert_replace Rename simdhash replace operation to make it clear that it only replaces the value Adjust modified code to have the same overwrite behavior as the old g_hash_table Probably meaningless semantic tweak for replace handler Migrate jit_code_hash and interp_code_hash to simdhash_ptr_ptr
This was referenced Jun 6, 2024
lambdageek
approved these changes
Jun 6, 2024
|
|
||
| for (guint i = 0; i < methods->len; ++i) { | ||
| m = (MonoMethod *)g_ptr_array_index (methods, i); | ||
| domain = (MonoDomain *)g_ptr_array_index (method_domains, i); | ||
| domain = user_data.domain; |
There was a problem hiding this comment.
Why do we need user_data.domain? can we just say domain = mono_get_root_domain(); or maybe change set_bp_in_method to call mono_get_root_domain() directly, if necessary (and I'm hoping it's not even necessary. We only really need to mention domains at the debugger protocol boundary, probably).
There was a problem hiding this comment.
Looking at the code, I think we should just delete domain from BreakpointInstance and then chase down all the places that touch that field.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Migrates various GHashTables in the mono runtime over to dn_simdhash, and optimizes a couple specific spots in the process.
There are more hash tables in mono that might be profitable to migrate based on profiling, but they're more complex scenarios, i.e. internal hash tables or concurrent hash tables, so I opted not to do them in this PR. In one case when I tried migrating one of them, things actually got slower.