Help wanted: Source Keepers overhaul
-
Dang, I didn't realize that your PRs had been out for a month, @PostCrafter. @artch any idea of when this will be tested/merged?
-
@semperrabbit It's not optimized in any ways yet (paths aren't getting saved to database yet) and there are a few other things I would change before merging it in. Due to my experience with past pull requests I don't feel like spending more time on this until I get some sort of official response whether my changes are going into the anticipated direction at all.
-
I'm sorry we can't afford spending time reviewing unfinished work, community contributions are supposed to save resources, not consume them. The direction seems to be fine (the only thing which concerns me is modifying
processor.js), but the devil is in the details, and I can't dive into the details right now, too many things require attention.
-
Do the source keepers still need an overhaul? Are they still a large portion of server load?
I sketched out the code and it's fairly straight forward, but I don't want to go through the hassle of a pull request if interest has waned or they're no longer a problem.
-
The code is still needed @deft-code
the reward is still up for grabs.
-
I've got a clone about ready for a pull request https://github.com/deft-code/engine/tree/keepers. The logic is all done, I just need to test it out, and get someone to review it.
Two things: First. The SourceKeeper:* users are configured outside of the engine. I'm planning to just ignore those users but continue using userID 3. This should be fine, but if the SourceKeeper users remain configured they'll still incur some runtime overhead, (Those users will throw an exception every tick when
require('main')is called by the engine). I could instead actively skip the SourceKeeper users, but it would cleaner to just assume they no longer exist. If the extra users will be an ongoing problem It might be worth it to add a check that skips running user code if there is not main module defined.Second: The current SourceKeeper and Invader code don't use a loop function. I suspect that might be a big part of their overhead. A less invasive fix would be to update their code to use a loop and possibly make all the SourceKeeper:* users share a single global (to avoid recompiling their code for each room).
If I were directing the project I'd try out the second option first, unless the plans are to move all the bot code into the processor. There is kind of a purity to coding the bots using the same API given to players and it would be nice to make that work better rather than bypass it.
-
Pull request is live. https://github.com/screeps/engine/pull/96
I'm still interested to know if using a main.loop with the keepers or invaders lowers their cpu burden. I talked with @ags131 who runs screepsplus (the largest private server) and he doesn't currently have the ability to monitor SK cpu burden or run a forked engine. It looks like the dev team might be only one who can verify this without some investing a lot of time.
If there is interest I can sent another PR that just updates the SK code to use a main.loop (very low risk), though honestly it'd probably just be easier for the devs if they made that small edit themselves.
-
Very interesting
-
Strongholds question: but very SK rewrite specific.
Is the stronghold code running in a VM or in the processor?\
I'm assuming in a VM.
Now that there is the concept of per sector bot. It might make sense to refactor the invader and SK code to run once per sector instead of once per room. I think that will lessen the burden on the servers (It seems like there is significant CPU and Memory overhead for running a use vm).
My SK rewrite didn't really have that much more efficient logic but it does bypass the vm overhead (compiled code, Memory, intent queueing). Amortizing the vm overhead across an entire sector might be enough to fix the server burden.
It would also let me give up on testing the SK logic. I'd much rather have bots using the player api than than getting my PR accepted.
On a side note I hate manual testing, I'd gladly split the sub token bounty(3 subs) with anyone willing to test the PR for me. Assuming of course the devs still want in processor SK logic.
-
@deft-code iirc @o4kapuk said on the slack discussion that the stronghold code will run on the processor and not will not be usable for player code in any way.
-
I read @o4kapuk's pull request. It looks like the stronghold logic is in the processor. I didn't see any like creep logic in the processor though.
-
Because it's still WIP
-
This has been deployed to PTR. Both @PostCrafter and @deft-code get 2 Subscription Tokens, since we partially utilized their groundwork in our development.
