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

feat(core/profiler): add natives exposing start/stop recording #2486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AvarianKnight
Copy link
Contributor

@AvarianKnight AvarianKnight commented Apr 25, 2024

  • currently trying to do recordings can only be done via console commands, which can be tedious
  • this adds PROFILER_START_RECORDING and PROFILER_STOP_RECORDING to allow automating profiler captures

Goal of this PR

Allow resources to start/stop recording to allow for easier automation of testing performance of code

How is this PR achieving the goal

Exposes PROFILER_START_RECORDING and PROFILER_STOP_RECORDING

This PR applies to the following area(s)

Natives, SCRT

Successfully tested on

Platforms: Windows

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Apr 25, 2024
@AvarianKnight AvarianKnight reopened this Sep 29, 2024
@github-actions github-actions bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Sep 29, 2024
@FabianTerhorst FabianTerhorst added enhancement Feature or other request that adds functionality or improved usability and removed invalid Requires changes before it's considered valid and can be (re)triaged labels Sep 30, 2024
@AvarianKnight AvarianKnight marked this pull request as draft October 2, 2024 16:57
@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Oct 2, 2024
@AvarianKnight AvarianKnight force-pushed the feat/profiler-natives branch 2 times, most recently from 3dd0208 to 6dad489 Compare October 25, 2024 19:52
@AvarianKnight AvarianKnight marked this pull request as ready for review October 25, 2024 19:59
@github-actions github-actions bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Oct 25, 2024
@AvarianKnight
Copy link
Contributor Author

AvarianKnight commented Oct 28, 2024

Here's some test code.

Note on the client you'll have to set profiler_allowSaveNatives true

CreateThread(function()
    local profilerStarted = false
    local profilerId = 0
    while true do
        if not profilerStarted then
            profilerStarted = true
            print("started")
            ProfilerStartRecording(50)
        elseif not ProfilerIsRecording() then
            print("save")
            profilerId = profilerId + 1
            local jsonFile = ProfilerSaveToJson()
            local msgPackFile = ProfilerSaveToMsgpack()
            print("json", jsonFile)
            print("msgpack", msgPackFile)
            profilerStarted = false
        end
        Wait(0)
    end
end)

- currently trying to do recordings can only be done via console commands, which can be tedious
- adds `PROFILER_START_RECORDING` and `PROFILER_STOP_RECORDING` to allow automating profiler captures
- adds `PROFILER_SAVE_TO_JSON` and `PROFILER_SAVE_TO_MSGPACK` to allow for automated saving of profiler recordings
@AvarianKnight
Copy link
Contributor Author

The implementation has swapped from taking a string value as the file name, to defining one based on the current time of day to not allow the functions to be used maliciously.

This also adds a profiler:/ vfs to save profiler recordings to so they wont set in the base directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature or other request that adds functionality or improved usability invalid Requires changes before it's considered valid and can be (re)triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants