-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
reduce script extensions loading time using threadpoolexecutor #16545
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for most script it's fine but similer to your other #16548, this will call scripts be loaded in undetermined order
some scripts are designed in a way that they are loaded in a specific order
otherwise it breaks
sometimes the order within an extension itself (IMO this means that the extension has multiple coded poorly) bout sometimes require other extensions to be loaded first
while this mainly affects the extensions that integrates with or dependent onother extensions, but those extensions do exist
also so even there is no conflicts within extensions
also
it is totally possible during loading of extension module
the extension decides to modify / patch parts of web UI
which is not thread safe
basically if the PR is meaged is that it will requires all extension loading to be rewritten with thread safe manner
and since this is not the case in the past, an extension has to declare that they are thread safe for us to load it in parallel
it is totally possible for something to be broken to the point prevents the UI from even launching
furthermore the behavior is undeterministic which basically makes debugging impossible
this feature is also enabled by default which is asking for more trouble
be97762
to
cc20bd2
Compare
you mean, some extensions have execution code while loading time and that could cause issue? yes. you are right, some extensions have execution code like as
any possible monkey patches should use proper hooks If he want to guarantee the intended behaviour.
we don't have to make perfect extensions, simply we can switch this feature on and off by setting |
conclusion it requires much much more work to allow the modules to be loaded in parallel |
Description
script_loading_max_thread
"System" option. (set max thread =1 for normal loading)Notes: Due to the fact that
sys.path
is required withLock()
, the effect ofmax_thread>2
is not quite large.Results:
test condition:
without
ThreadPoolExecutor()
:script_loading_max_thread=1
~16.5 sec
load scripts
time.with
ThreadPoolExecutor()
:script_loading_max_thread=4
~10 sec
load scripts
time.Checklist: