ggrossetie / asciidoctor-web-pdf

Convert AsciiDoc documents to PDF using web technologies
https://asciidoctor.org
MIT License
450 stars 92 forks source link

Pr fix readline issue #488

Closed wh81752 closed 3 years ago

wh81752 commented 3 years ago

This is a fix for the on-going readline() issue of consuming 100% CPU on running locally npm run build.

The fix itself is rather straight-forward. Usage of readline() is completly dropped. Instead ANSI escape chars are used (like mentioned here https://tforgione.fr/posts/ansi-escape-codes):

The relevant section now looks like

   await puppeteer
      .createBrowserFetcher({..})
      .download(puppeteer._preferredRevision, function (downloadBytes, totalBytes) {
        const percent = Math.round(downloadBytes / totalBytes * 100)
        console.log('\x1B[1A\x1B[K' + `Downloading browser for ${name.padEnd(5)} ${percent.toString().padStart(5)}%`)
      })

Here \x1B[2K erases the whole line and also moves the cursor to start of (erased line).

This change works perfectly well on my local MacOS machine, whether running inside IntelliJ's terminal or when using a regular xterm terminal. Download takes time in, well, almost no time while before the CPU consumed all CPU power and finished in about 20 minutes per platform.

ggrossetie commented 3 years ago

Nice work! :tada:

It seems faster on my machine too 😄 Do you know if it would be possible to get one line per platform?

Downloading browser for linux    55%
Downloading browser for win      27%
Downloading browser for mac      12%

I did some tests with \x1B[s (save cursor position) and \x1B[u (restore cursor position) but to no avail... It might be tricky because downloads are executed in parallel so each "thread" will need to save the cursor position.

Reference: https://gist.github.com/fnky/458719343aabd01cfb17a3a4f7296797

Anyway, it's fine even if it's not possible, that's already much better!

wh81752 commented 3 years ago

I guess the overall problem of ANSI escape chars is portability.

To overcome libs like readline() and sync-readline() make sense and provide a value.

I could not really figure out what the problem with the former code is. It looked pretty good to me, nonetheless, when running locally it looks like I'm doing some heavy crypto mining :-)

It might be tricky because downloads are executed in parallel so each "thread" will need to save the cursor position.

Not really. My approach would be to move the cursor back into its initial position (char zero at line zero) after every update. Then all you need to do is

  1. to calculate the line number
  2. move cursor to line X
  3. erase and rewrite line X
  4. move cursor back to initial position

I can try, just for fun.

On Wed, May 12, 2021 at 2:28 PM Guillaume Grossetie < @.***> wrote:

Nice work! 🎉

It seems faster on my machine too 😄 Do you know if it would be possible to get one line per platform?

Downloading browser for linux 55%

Downloading browser for win 27%

Downloading browser for mac 12%

I did some tests with \x1B[s (save cursor position) and \x1B[u (restore cursor position) but to no avail... It might be tricky because downloads are executed in parallel so each "thread" will need to save the cursor position.

Reference: https://gist.github.com/fnky/458719343aabd01cfb17a3a4f7296797

Anyway, it's fine even if it's not possible, that's already much better!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Mogztter/asciidoctor-web-pdf/pull/488#issuecomment-839730171, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB42TPKXQIUOC7YVCOQTRWLTNJX6DANCNFSM44YOLTZA .

-- Wolfgang Häfelinger häfelinger IT - Applied Software Architecture http://www.haefelinger.it +49 1520 32 52 981 (+31 648 27 61 59)

ggrossetie commented 3 years ago

Closing in favor of #490