alliedmodders / metamod-source

Metamod:Source - C++ Plugin Environment and Detour Library for the Source Engine
http://www.metamodsource.net/
Other
374 stars 83 forks source link

Replace fscanf with faster parsing #127

Closed PeakKS closed 11 months ago

PeakKS commented 11 months ago

Sorry I had to duplicate this, turns out there's no way to change the head branch on a PR. Refer to previous PR #108 and the discussion about this on the AM discord here.

psychonic commented 11 months ago

Did you experience performance problems with the current code and did those benchmarks using some SourceHook testing plugin or unit test? Complicating the code as a pre-mature optimization doesn't seem sustainable. It does sound probable that this code path might be slow with lots of quick hook/unhooks, but measurements would help.

Was the comment by @peace-maker above from the previous MR addressed?

PeakKS commented 11 months ago

Did you experience performance problems with the current code and did those benchmarks using some SourceHook testing plugin or unit test? Complicating the code as a pre-mature optimization doesn't seem sustainable. It does sound probable that this code path might be slow with lots of quick hook/unhooks, but measurements would help.

Was the comment by @peace-maker above from the previous MR addressed?

In that discord thread we found that Linux srcds entity creation behaves differently than windows. With light entities for example each one is created and then immediately destroyed meaning if all entities are hooked the hook is repeatedly created and destroyed. Vauff tested this PR in his server and reported that maps with long freezes on round change had those freezes cut in half, matching with the results of my benchmarking.