search

pfalstad / circuitjs1

Electronic Circuit Simulator in the Browser
GNU General Public License v2.0
1.65k stars 280 forks source link

Introduce a new Performance Monitor and Developer Mode #20

Closed Mervill closed 2 years ago

Mervill commented 2 years ago

NOTE: It'll be easiest to view this PR with whitespace ignored. (quick link)

This PR introduces a new PerformanceMonitor that uses a high precision timer to get a better view of how the sim is performing. The monitor can be split into different contexts that can be nested.

2022-04-14 22_37_24-127 0 0 1_8888_circuitjs html

updateCircuit(): 2.99   // The whole sim took 2.299 ms
-runCircuit(): 0.2      // This function took 0.2 ms
-graphics: 2            // This is showing the total of the following two child times (2ms)
--elm.draw(): 0.2       // Took 0.2ms to draw all the elements
--drawBottomArea(): 1.7 // Took 1.7ms to draw the scopes

Additionally, this developer info can now be togged without recompiling via a new option in the settings.

2022-04-14 22_43_19-127 0 0 1_8888_circuitjs html

This PR is very much in line with the kind of contributions I want to make to the project, refactoring + new small features where it makes sense.

@pfalstad would you consider opening the issues page for this fork of the project? Using the sharpie7 issues page dosen't seem appropriate.

Mervill commented 2 years ago

@pfalstad Comments or Concerns on this pull request? I want to contribute however I can so if you don't like something about this pull please let me know!

pfalstad commented 2 years ago

Sorry, I forgot about this. Thanks, this would have been handy when I was working on my pong project! I wasn't sure if I wanted a checkbox in the options, but that is really a small part of it and not a big deal. I have merged this into my local sandbox and it will show up next time I release.

pfalstad commented 2 years ago

using sharpie7 issues page is weird but that's where everything is, and I don't want to check two places.

Mervill commented 2 years ago

I'm working on expanding this, so merging via github will let me prepare new pull requests without having to worry about rebase problems further down the line. Let me know if you have any concerns about merge conflicts with your local changes.

Regarding the Developer Mode option: Commented out code is impossible to maintain, so if you'd prefer not to have an option in the menus, simply having a developerMode Boolean in the main class would be sufficient. Eventually I hope developer mode will expose a bunch more info that will make characterizing changes to the internals much easier, so having it all behind a coinvent switch is essential.

Mervill commented 2 years ago

@pfalstad Hey thanks! I'm looking forward to contributing more to the project. The last thing I want to do is step on your toes or overwhelm you with stuff (we're both doing this on the side, after all) so don't hesitate to tell me if I need to cool my jets. Collaboration is key, after all.