stable-diffusion-webui
stable-diffusion-webui copied to clipboard
Hotfix: API cache cleanup
Description
After processing image generation via API p.close()
wasn't being called, and accordingly the cache wasn't being cleared as discussed on https://github.com/Mikubill/sd-webui-controlnet/issues/1719
This PR calls p.close()
after img2img and txt2img generations to do the necessary cleanup for API calls.
I hope we can merge this as a hot fix to master as well since it affects API integrations with Controlnet at the moment.
Checklist:
- [x] I have read contributing wiki page
- [x] I have performed a self-review of my own code
- [x] My code follows the style guidelines
- [x] My code passes tests
please clean the code first then you have to start fixing
Are you using experimental_persistent_cond_cache
? I'm not sure how this would be an issue otherwise..?
Are you using
experimental_persistent_cond_cache
? I'm not sure how this would be an issue otherwise..?
I'm not using experimental_persistent_cond_cache
, however, if you look at __init__
:
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/master/modules/processing.py#L182
self.cached_uc = StableDiffusionProcessing.cached_uc
self.cached_c = StableDiffusionProcessing.cached_c
It's self referencing, so, without running close()
after processing, the values won't be reset at:
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/master/modules/processing.py#L295C9-L297C63
if not opts.experimental_persistent_cond_cache:
StableDiffusionProcessing.cached_c = [None, None]
StableDiffusionProcessing.cached_uc = [None, None]
Before https://github.com/AUTOMATIC1111/stable-diffusion-webui/commit/7f2214aa2b9a88bc1631f3f1d164a75a3583c851#diff-1fe5b15d29310b37af0c4560da365990f4e6eee7b99bf3f041cfa18ebb11610fL179, both values where reset in the __init__
function explicitly:
self.cached_uc = [None, None]
self.cached_c = [None, None]
In short, without calling p.close()
after processing an API call, the cleanup won't happen; even when experimental_persistent_cond_cache
is not set.
Ping @w-e-w, can you take a look since you authored the conds cache stuff? ☝️
yes I think this is a bug in API
apparently conds cacheis not cleare wheh using API because it's missing p.close
in the flow
effectively it's like having experimental_persistent_cond_cache
always enabled
so this PR will fix the issue
the question is now how do we make experimental_persistent_cond_cache
compatibleis with controlnet
irrc before the addition of experimental_persistent_cond_cache
p.close
was redundant, so there was no issue with api not haveing p.close
when I make the change I didn't check if API had p.close
, I assume that it had that like in the gradio pipeline
apparently I found a bug /design flaw in GitHub projects If I add this PR to project (even to my personal) and change the title of the project entry, it will also change the title here no confirmation
For a future refactor, we should probably use p
s with contextlib.closing()
for automagical cleanup :)
At least I'll suggest
p = StableDiffusionProcessingTxt2Img(sd_model=shared.sd_model, **args)
try:
p.scripts = script_runner
p.outpath_grids = opts.outdir_txt2img_grids
p.outpath_samples = opts.outdir_txt2img_samples
shared.state.begin()
if selectable_scripts is not None:
p.script_args = script_args
processed = scripts.scripts_txt2img.run(p, *p.script_args) # Need to pass args as list here
else:
p.script_args = tuple(script_args) # Need to pass args as tuple here
processed = process_images(p)
shared.state.end()
finally:
p.close()
Instead of just p.close(), it should expect some failure can happen during operation (mostly OOM /CUDAError), then still try to close it at last.
Contextlib is more clean version for this.
At least I'll suggest
p = StableDiffusionProcessingTxt2Img(sd_model=shared.sd_model, **args) try: p.scripts = script_runner p.outpath_grids = opts.outdir_txt2img_grids p.outpath_samples = opts.outdir_txt2img_samples shared.state.begin() if selectable_scripts is not None: p.script_args = script_args processed = scripts.scripts_txt2img.run(p, *p.script_args) # Need to pass args as list here else: p.script_args = tuple(script_args) # Need to pass args as tuple here processed = process_images(p) shared.state.end() finally: p.close()
Instead of just p.close(), it should expect some failure can happen during operation (mostly OOM /CUDAError), then still try to close it at last.
Contextlib is more clean version for this.
Good call, I pushed the changes.
Can we merge this PR?
Can we merge this PR?
AUTO has gone into hibernation