Page MenuHomePhabricator

don't extra notify ModulesDidLoad() from LoadModuleAtAddress()
ClosedPublic

Authored by llunak on Apr 5 2022, 6:52 AM.

Details

Summary

Places calling LoadModuleAddress() already call ModulesDidLoad() after a loop calling LoadModuleAtAddress(), so it's not necessary to call it from there, and the batched ModulesDidLoad() may be more efficient than this place calling it one after one.

This also makes the ModuleLoadedNotifys test pass on Linux now that the duplicates no longer bring down the average of modules notified per call.

This change is a prerequisite for parallelizing PreloadSymbols() in D122975.

Diff Detail

Event Timeline

llunak created this revision.Apr 5 2022, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2022, 6:52 AM
llunak requested review of this revision.Apr 5 2022, 6:52 AM
labath added a subscriber: labath.Apr 5 2022, 7:46 AM

Note that this patch could have been shorter if I simply changed LoadModuleAtAddress() to always pass false for notify (all callers currently call ModulesDidLoad() afterwards) and added a note to LoadModuleAtAddress() docs about it, but I don't know if that's a good design choice.

That would definitely be better.

llunak updated this revision to Diff 421782.Apr 10 2022, 4:56 AM
llunak edited the summary of this revision. (Show Details)

Changed to always disable notify and added a comment about that to LoadModuleAtAddress().

labath accepted this revision.Apr 11 2022, 5:39 AM

Cool. Thanks.

This revision is now accepted and ready to land.Apr 11 2022, 5:39 AM