Skip to content

Speed up server startup by running asset compilation in parallel - #3093

Closed
davestgermain wants to merge 1 commit into
masterfrom
dcs/quicker-startup
Closed

Speed up server startup by running asset compilation in parallel#3093
davestgermain wants to merge 1 commit into
masterfrom
dcs/quicker-startup

Conversation

@davestgermain

Copy link
Copy Markdown
Contributor

On my machine, the devstack server starts up 10 seconds faster.

@singingwolfboy @wedaly

@davestgermain davestgermain changed the title Attempt to speed up server startup by running asset compilation in parallel Speed up server startup by running asset compilation in parallel Mar 27, 2014
@singingwolfboy

Copy link
Copy Markdown
Contributor

Seems like there's no way to use keyword arguments with the solution you've proposed. Also, under what circumstances do you expect this to run with concurrent.futures.ThreadPoolExecutor vs threading.Thread?

@davestgermain

Copy link
Copy Markdown
Contributor Author

@singingwolfboy the functions didn't need kwargs, but I can adapt it if you want.
The function would run with concurrent.futures if we decide to add that library (it's available as a backport), or when we move to python 3 someday...

@singingwolfboy

Copy link
Copy Markdown
Contributor

@davestgermain yeah, would you mind adding kwargs support? I want it to be easy to use this run_threaded function for other things, and I think this will help make it easier.

Comment thread pavelib/utils/thread.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

max_workers=4 seems a fairly arbitrary value. Is there a way we can select a value more intelligently? If not, I'm OK with just setting it to 4, but it's worth asking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

4 seemed reasonable given the number of cpus and the fact that most tasks will be IO bound. I believe 4 is the default, anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we do something like:

try:
    import multiprocessing
    num_cpus = multiprocessing.cpu_count()
except ImportError:
    # reasonable default
    num_cpus = 4

with ThreadPoolExecutor(max_workers=num_cpus) as executor:
    # etc

@davestgermain

Copy link
Copy Markdown
Contributor Author

@singingwolfboy added kwargs support and cpu_count (multiprocessing is always available in python 2.7)

@davestgermain

Copy link
Copy Markdown
Contributor Author

@singingwolfboy Do have any guesses why this change would cause the acceptance test failures? Could it be that there's an order dependency when compiling coffeescript or sass? I made a change assuming that was the case, but the tests still failed.

@singingwolfboy

Copy link
Copy Markdown
Contributor

Not sure. Maybe the tests are starting up before all the assets are finished being compiled?

@singingwolfboy

Copy link
Copy Markdown
Contributor

This pull request needs to be rebased. Also, there's some discussion about moving from paver to invoke (see #3444) so perhaps this PR is no longer useful?

@davestgermain

Copy link
Copy Markdown
Contributor Author

I never could figure out why the tests were failing. should I remove this PR?

@singingwolfboy

Copy link
Copy Markdown
Contributor

@davestgermain that seems reasonable

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