plotly / Kaleido

Fast static image export for web-based visualization libraries with zero dependencies
Other
363 stars 36 forks source link

Attempt to fix static image export hanging by flushing after printing JSON #149

Open adeak opened 1 year ago

adeak commented 1 year ago

This is a potentially "wishful thinking" attempt to fix (a subset of?) the "writing image stalls" problem with a dozen or so open issues. Unfortunately I haven't been able to test whether it fixes the issue, because I do not have the tooling at hand to build the library (assuming this indeed involves the 50 GB chromium compile). If there's a low-profile way to compile this patch I'd love to test it locally on my "known bad" setup.

My situation:

  1. A coworker and I have almost identical hardware, identical OS version (down to the exact Windows build) and the exact same conda environment. His conda is 4.8 and mine is 4.11, but other than this I couldn't pin down any differences between our setups. For the time being we're stuck with plotly 4.11.0.
  2. Any call to fig.write_image() will hang on my machine, and it will always succeed on coworker's machine. (So it's not the "sometimes hangs on the same system" case some people see.)
  3. I get a hang in any shell I can find: git-bash, cmd, powershell, even WSL. (So it's not the "doesn't work in WSL only" case some people see.)
  4. The hang always happens in the usual suspect self._proc.stdout.readline() line in _ensure_kaleido(), see e.g. traceback in https://github.com/plotly/Kaleido/issues/36#issuecomment-756614157
  5. Running the subprocess' corresponding command directly in the shell gives the same output as https://github.com/plotly/Kaleido/issues/110#issuecomment-924929419, where the stdout JSON payload even has a trailing newline as it should.
  6. Disabling mathjax doesn't help. Nor does disabling internet entirely. And the hang never ended during the 3-hour slot I let it run once. (So it's probably not the "kaleido waits for timeout" case some people see.)

Buffering?

The only thing I can suspect here is buffering. If for some reason the process is not line buffered (and kaleido runs the subprocess with a binary stdout stream, so I'm not even sure line buffering is a thing there), the readline() call might be stuck, waiting for the stdout stream to flush, which for some reason might not happen on some systems.

Why it might not be buffering:

  1. It's quite surprising if two essentially identical systems (those of coworker and I) have markedly different buffering behaviour.
  2. The newline seems to be printed fine, which normally should flush assuming line buffering (but, again, this is a binary stream for now).
  3. Editing the library to open the subprocess with text mode streams via universal_newlines=True doesn't make the problem go away. Manually calling self._proc.stdout.flush() doesn't make the problem go away. Accessing self._proc.stdout.raw which might be less buffered doesn't make the problem go away. Calling self._proc.stdout.read(70) or even .read(50) (since we expect 70 characters for the JSON) halts all the same.
  4. Trying to reproduce the issue running a small python script that prints a similar linefeed-terminated line (run through the same subprocess.Popen setup) doesn't stall.
  5. Editing the library to open the subprocess with bufsize=50 (say) which is shorter than the 70 bytes of the expected JSON doesn't make the problem go away (it still stalls).

Why it might be buffering:

  1. This would explain why the readline() call freezes in the subprocess call (JSON is stuck in the buffer).
  2. The JSON writer adds the linefeed to the string rather than calling std::endl which would flush.
  3. Frankly, I have no other idea and the manual flush in this patch seems harmless enough to try, considering that the mysterious issue has been around for a long time.

Unfortunately I couldn't find any exact information on what kind of buffering options are available on a given OS, and what defaults are. The only thing I could figure out is that both in my native Windows and in my WSL the io.DEFAULT_BUFFER_SIZE is 8192, so if the output is not line buffered then it would take a lot of output to get it flushed (assuming I even understand the mechanics here correctly).

Lucas-C commented 1 year ago

Hi @adeak

I may be facing the same issue in https://github.com/PyFPDF/fpdf2/pull/714, and I'd love to try your patch. Considering that the patch applies to kaleido/cc/utils.h, will it be taken into account if install Kaleido from your git branch?

pip install git+https://github.com/adeak/Kaleido.git@flush_after_json_printing

Or do I have to compile my own version of the kaleido binary executable?

adeak commented 1 year ago

That won't work @Lucas-C, because the package has no setup.py, and because it's probably not how it works in general.

Reading the README (especially the "disadvantages" section) sounded like compilation is a huge amount of effort. This is why I didn't try to do that... Even if this PR works I'd have to build kaleido twice: first to check that a local rebuild on its own doesn't make the issue go away, and then rebuild for the PR branch. And it's very likely that

  1. the attempted fix doesn't work, because the issue is not buffering after all, or
  2. the fix works but then the code gets stuck on the next print, somewhere down the line.

Considering that the library seems unmaintained in general, this is not a super encouraging situation for going on potential wild goose chases, alas. When I opened this PR I was hoping that someone with the project already has tooling and CPU hours to build this so we can test.

Lucas-C commented 1 year ago

Thank you for the detailed answer @adeak!

I tried to compile it on my machine yesterday, following the steps in the .circleci/config.yml, win_scripts/fetch_chromium.ps1 & win_scripts/build_kaleido.ps1 files, but it took ages to download & setup all the required repos & tools, so I gave up in the end.

I probably won't dive into this any further.

Whoever reads this: have a nice day!

Connor-Heindel commented 6 months ago

I pulled the latest version and the fork into a local repo and went through all of the compilation steps, and installed the generated wheel file on my WSL2 setup, which is having the same issue, and unfortunately I had no luck with it. It was still hanging on image generation forever, and the error stream when interrupted was the same.

The next step I'd like to try would be pulling a newer version of Chromium, since this current Kaleido build uses one that is significantly older, but it requires re-tooling the majority of the build process, and won't be a fast process to find all of the changes that have to be made.

gvwilson commented 2 months ago

Thanks for your interest in Kaleido. We are currently working on an overhaul that might address your issue - we hope to have news in a few weeks and will post an update then. Thanks - @gvwilson

adeak commented 2 months ago

Sounds great, thanks for the update @gvwilson.