Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace old profiler with new one #61

Closed
wants to merge 4 commits into from

Conversation

bryanbecker
Copy link
Member

This change is probably controversial, so lets hear feedback.

I ripped out the old profiler, and added https://github.com/screepers/screeps-typescript-profiler

It's set up with everything it needs to work, but commented out by default, since the starter doesn't include anything profilable (only classes and class-methods can be profiled).

Also updated the readme to reflect the repo needs to be cloned with --recursive tag

@KarateSnoopy
Copy link

KarateSnoopy commented Aug 6, 2017

btw, I pulled this change into my fork at https://github.com/KarateSnoopy/screeps-typescript-starter/tree/profiler-merged

My changes:

  1. fixed the minor conflict in main.ts

  2. I also had to change the submodule in .gitmodules to point to https://github.com/screepers/screeps-typescript-profiler.git so I could use "git submodule update --init"

  3. I also updated the readme, since you should note that submodules aren't including in the .zip on github so you'll need to tell folks to clone --recursive or fetch run the git submodule update --init command.

  4. For some reason, if I use the latest profiler submodule commit, I was getting an error in

[at-loader] ./src/lib/Profiler/Profiler/Profiler.ts:43:5
TS2322: Type '{ clear(): string; output(): string; start(): string; status(): "Profiler is running" | "Profiler...' is not assignable to type 'Profiler'.
Object literal may only specify known properties, and 'toString' does not exist in type 'Profiler'.

This pull request https://github.com/screepers/screeps-typescript-profiler/pull/1/files added this line.
Not sure why it errors for me.

  1. Changed
    // global.P = Profiler.init();
    to
    // global.Profiler = Profiler.init();
    so the profiler readme's commands work without change.

  2. But I'm having trouble using @profile. When I add that to creepManager's run function like so:

@profile
export function run(room: Room): void {
_loadCreeps(room);

Then I get this error which I can't figure out how to shake.

TS1206: Decorators are not valid here.

But I can get it working by doing it manually. For example:

function mainLoop() {
const start = Game.cpu.getUsed();
Main.go();
const end = Game.cpu.getUsed();
record("Main.go", end - start);
}

Any idea why @profile doesn't work? Thanks

@resir014
Copy link
Member

@KarateSnoopy I haven't tried the typescript profiler, but I'm planning to migrate to it in my primary codebase. @bryanbecker seems to be inactive as of late, so I'll have to figure things out on my own atm. Will let you know if I find anything.

Regardless, his work on the new profiler looks promising. I'm considering merging this PR if I could get it to work on my primary codebase.

@resir014
Copy link
Member

@KarateSnoopy Alright, I seem to have found what caused your issues.

As mentioned earlier, I've created a fork to attempt to fix many of the issues found in the profiler, so if you want, you can use that until the upstream is fixed.
https://github.com/resir014/screeps-typescript-profiler

@resir014
Copy link
Member

Superseded with future plans of revamping the starter kit.

@resir014 resir014 closed this Nov 21, 2017
@resir014 resir014 removed the backlog label Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants