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

Add obelisk profiling feature #654

Merged
merged 45 commits into from
Mar 13, 2020
Merged

Add obelisk profiling feature #654

merged 45 commits into from
Mar 13, 2020

Conversation

matthewbauer
Copy link
Collaborator

@matthewbauer matthewbauer commented Feb 11, 2020

Adds obelisk profile command. This dumps some profiling information into the profile/ directory that can give you some idea of what your program is spending resources on. Currently outputs 3 profiles:

  • Heap profiling (.hp)
  • Time profiling (.prof)
  • Reflex profiling (.rprof)

I have:

  • Based work on latest develop branch
  • Looked for lint in my changes with hlint . (lint found code you did not write can be left alone)
  • Run the test suite: $(nix-build -A selftest --no-out-link)
  • (Optional) Run CI tests locally: nix-build release.nix -A build.x86_64-linux --no-out-link (or x86_64-darwin on macOS)

This makes it easier to add ‘Ob run’-like commands by factoring out
important bits.
This adds profiled attr to set enableLibraryProfiling to true. If not
set, it defaults to profiling arg.
This dumps some profiling information that can be viewed and used to
debug Obelisk projects. ‘ob profile’ works like ob run, but instead of
using ghci, it builds an executable that is built with profiling enabled.
This avoids polluting the root directory and gives some ability to
reference multiple profiles.
@matthewbauer matthewbauer changed the base branch from master to develop February 11, 2020 17:12
@matthewbauer matthewbauer requested a review from 3noch February 11, 2020 17:12
This avoids a call every .1 seconds.
This module will always be there, and no need to add annotations.
backend & frontend still get their annotations.
This is unnecessary as we import immediately in the expression.
This avoids an issue with precedence in case of obRunExpr using $.
This avoids having to mix up options for ob run and ob profile. Makes
the way for adding a -o option to ob profile.
@matthewbauer
Copy link
Collaborator Author

I'm still pretty suspicious of bothering to find root here. What's the problem if we just use CWD again?

It seems safer to use an absolute path here. I originally assumed it would save it to the executable's directory, not the CWD. That's not the case, but the documentation leaves that behavior unspecified (https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/profiling.html#rts-flag--po%20%E2%9F%A8stem%E2%9F%A9), and I'd prefer to not have to debug it if it ever changes.

This allows custom flags to be provided by the user. For instance,
there are multiple types of heap profiles that can be used to get
different information from ob profile. These can be added with the
--rts-flags option.
This makes the default more obvious in the command line options.
- Use list of strings for rts args
- Move time handling to the profile function, out of argument handling
@@ -1,8 +1,8 @@
{
"owner": "reflex-frp",
"repo": "reflex-platform",
"branch": "master",
"branch": "bump-reflex-0-6-4-1",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this is merged we should bump and then we can merge here.

3noch and others added 3 commits February 20, 2020 11:59
This file is just a static nix script. We can avoid hereDoc and just
hit the attr directly. In addition, add the skeleton’s profiledObRun
to the release.nix.
default.nix Outdated Show resolved Hide resolved
matthewbauer and others added 5 commits February 21, 2020 15:45
pinBuildInputs tries to run binaries when they exist. We don’t want
that, so we need to wrap it in a directory with $out/bin/ob-run
containing the actual binary.
$out/bin/ob-run was a mistake
@3noch 3noch merged commit a3ccae6 into develop Mar 13, 2020
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.

2 participants