[mono] Migrate more ghashtables to simdhash #102476
Conversation
ghost added the 
kg requested review from BrzVlad, kotlarmilos, lambdageek, marek-safar, steveisok and thaystg as code owners 
lambdageek left a comment
lambdageek left a comment There was a problem hiding this comment.
lgtm, but I think we should get rid of the debugger engine hash table that's keyed by domain
(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
lambdageek left a comment
lambdageek left a comment There was a problem hiding this comment.
looks good, but if you want to delete more MonoDomains from the debugger, please do it
| 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.
2 participants
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.