Improve daemon run profiling.

Review Request #4293 — Created Oct. 11, 2016 and submitted

kwlzn
pants
3944
pants-reviews
nhoward_tw, stuhood, yujiec

This change adds a new PANTSD_PROFILE env var for profiling the daemon-side execution of a pants invocation and repairs PANTS_PROFILE in the daemon runner context with a deferred approach to calling os._exit() at the end of the forked child to permit the profiling data to be written to disk.

Combining these like so:

$ PANTSD_PROFILE=/tmp/daemon.prof PANTS_PROFILE=/tmp/runner.prof ./pants ...

will cause two profile dumps to be written representing the daemon-side and runner-side execution for more effective E2E profiling of daemon runs.

dogfooded this for the generated profiles in #3929 + CI is green here: https://travis-ci.org/pantsbuild/pants/builds/166878757

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
NH
  1. 
      
  2. src/python/pants/util/contextutil.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     

    Could you add a test for this?

    Also, it would be good to clarify that HardSystemExit exits with a 0 status code.

    How common knowledge is os._exit()? Should this note that it causes exit handlers to be skipped?

    1. added a test and a quick nod to the 0 status code.

      os._exit() is part of the stdlib and fully doc'd. I'd prefer to hang the concern of documenting behavior on the implementors rather than denormalizing docs all over the place - and hopefully the "hard exit" verbiage also helps make this clear.

    2. Thanks.

      I'm not suggesting denormalizing docs generally. Your NB comment contained info I thought might be worth including in the docstring for the used handlers too.

  3. 
      
KW
KW
YU
  1. Looks good! I have 1 question.

  2. Is it necessary to have hard_exit_handler here?
    It seems self._run() will not throw HardSystemExit.

    1. any runs invoked via the pailgun service should utilize DaemonExiter as a replacement for Exiter in DaemonPantsRunner and into its inner LocalPantsRunner here: https://github.com/pantsbuild/pants/blob/master/src/python/pants/bin/daemon_pants_runner.py#L157

      so in the case of a daemon run, any calls to exiter.exit() should result in marshalling of the exit code across the nailgun session + raising of HardSystemExit.

    2. Ah, right! Thanks for the clarification!

  3. 
      
NH
  1. 
      
  2. src/python/pants/util/BUILD (Diff revision 2)
     
     

    nit: sort

  3. 
      
KW
KW
Review request changed

Status: Closed (submitted)

Change Summary:

thanks Nick & Yujie! submitted @ f1bcf5d20502a1bd99211716fc2426dfb1268fec

Loading...