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

feature request: support --jobs option for precompile command #24

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@hhatto
Copy link

hhatto commented Aug 22, 2018

Hi,

precompile command is nice feature.
I want to improve the performance when I run precompile command.
Because there are over 300 target files in my use case.

So I added the --jobs option.
This option improves performance by running compile_one in parallel using multiprocessing.Pool.

execution result:

$ time CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir ~/template_files --jobs 1
INFO:root:Compiled 318 out of 318 found templates
CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir   1  63.78s user 1.04s system 92% cpu 1:09.71 total
$ rm -rf ~/tmp_chameleoncache/*
$ time CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir ~/template_files --jobs 2
INFO:root:Compiled 318 out of 318 found templates
CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir   2  78.24s user 1.07s system 172% cpu 45.959 total

Thanks

@stevepiercy

This comment has been minimized.

Copy link
Member

stevepiercy commented Aug 22, 2018

@hhatto please review requirements for making substantive contributions for all projects under the Pylons Project:
https://pylonsproject.org/community-how-to-contribute.html

Specifically:

  • All code must include tests that pass.
  • Narrative documentation should be included when the contributions conflict with existing documentation
  • Include an entry in CHANGES.txt
  • Add your name to CONTRIBUTORS.txt

Let us know if you have any questions.

Hideo Hattori added some commits Aug 22, 2018

Hideo Hattori
Hideo Hattori
Hideo Hattori
@@ -1,6 +1,8 @@
0.4 (unreleased)
----------------

- Add new command ``pyramid-chameleon-precompile``.

This comment has been minimized.

@hhatto

hhatto Aug 22, 2018

Strictly it is not my change, but I thought it was necessary, so I added it 😄

This comment has been minimized.

@bertjwregeer
@hhatto

This comment has been minimized.

Copy link

hhatto commented Aug 22, 2018

@stevepiercy

Thanks for reply.

I made some changes. How about this?

@stevepiercy stevepiercy requested review from bertjwregeer , mmerickel and mcdonc Aug 22, 2018

@stevepiercy

This comment has been minimized.

Copy link
Member

stevepiercy commented Aug 22, 2018

Thank you @hhatto. Would you feel comfortable adding some narrative text to the documentation under Precompiling? If not, I can add it later, but it would be good to include it with this PR. Here's my suggestion (line wrapping at 79 columns is not required in narrative documentation):

However compilation is relatively slow compared with execution, so the
first time a view is rendered may be very slow. To work around this
first request latency we offer a command line script to pre-compile
templates to python.
To further speed up the precompilation, use the ``--jobs <integer>`` option with an integer specifying the number of parallel jobs to run on a multiprocessor computer.
This is executed as follows:

.. code-block:: bash
    $ CHAMELEON_CACHE=/path/to/put/precompiled/templates \
        pyramid-chameleon-precompile --dir /path/to/look/for/templates \
        [--jobs <integer>]

With the ``--jobs`` option, the duration of compilation can be reduced by about one-third in projects with over 300 templates, as in this example using ``time``.

.. code-block:: bash

    $ time CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir ~/template_files --jobs 1
    INFO:root:Compiled 318 out of 318 found templates
    CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir   1  63.78s user 1.04s system 92% cpu 1:09.71 total
    $ rm -rf ~/tmp_chameleoncache/*
    $ time CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir ~/template_files --jobs 2
    INFO:root:Compiled 318 out of 318 found templates
    CHAMELEON_CACHE=~/tmp_chameleoncache pyramid-chameleon-precompile --dir   2  78.24s user 1.07s system 172% cpu 45.959 total

I've requested review from the maintainers. At least one of them should review this PR before merging.

@hhatto

This comment has been minimized.

Copy link

hhatto commented Aug 23, 2018

Would you feel comfortable adding some narrative text to the documentation under Precompiling?

It is good 👍
I will add it in this PR.

@stevepiercy
Copy link
Member

stevepiercy left a comment

This looks good to me.

One of the maintainers should review, too, and decide whether to cut a release. Ping @mcdonc @mmerickel @bertjwregeer

@bertjwregeer
Copy link
Member

bertjwregeer left a comment

fail_fast is no longer used/available... I am not sure this is what is intended.

@@ -1,6 +1,8 @@
0.4 (unreleased)
----------------

- Add new command ``pyramid-chameleon-precompile``.

This comment has been minimized.

@bertjwregeer
template = template_factory(fullpath, macro=None)
template.cook_check()
try:
assert chameleon.config.CACHE_DIRECTORY is not None

This comment has been minimized.

@bertjwregeer

bertjwregeer Sep 6, 2018

Member

I'd prefer this to be done once, rather than over and over again in a function that is going to be called many times.

This comment has been minimized.

@hhatto

hhatto Sep 19, 2018

This is the same as the original processing. (link to source code)

I assume that I use it as a single method of a module, not as a command, and I think that chameleon.config.CACHE_DIRECTORY is being checked here.
And, I think assert chameleon.config.CACHE_DIRECTORY is not None is low overhead so I do not need to delete it in particular.

directory,
extensions=frozenset(['.pt']),
template_factory=PyramidPageTemplateFile,
fail_fast=False,

This comment has been minimized.

@bertjwregeer

bertjwregeer Sep 6, 2018

Member

Where is fail_fast used in this function? If it is no longer used/useful then maybe it should be removed.

This comment has been minimized.

@hhatto

hhatto Sep 19, 2018

fail_fast is passed as an argument to use in _compile_one function.

@hhatto

This comment has been minimized.

Copy link

hhatto commented Sep 19, 2018

@bertjwregeer

Thanks for review comments.
I reply and fixing the source code, so can you confirm it again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment