pygments / pygments.rb

💎 Ruby wrapper for Pygments syntax highlighter
MIT License
572 stars 141 forks source link

Pipe #33

Closed tnm closed 12 years ago

tnm commented 12 years ago

This adds a full implementation of the Pipe Based Method of talking to Pygments. It maintains full API compatibility with the existing pygments.rb library.

The basic data flow:

  1. Rubyland opens a pipe to the mentos.py process if one does not already exist
  2. mentos listens for a fixed 32-length string, representing an integer, which represents header length
  3. Rubyland sends over a JSON header of the 'RPC'-ish method name, args, kwargs, and length of text to come (if any)
  4. Rubyland sends over the text, if any
  5. mentos returns the requested data

This pull also includes handling for dealing with dead child processes, catching SIGCHLD and then issuing the subsequent wait. We are also able to re-spawn child processes if they die. We also catch relevant errors involved in the syscalls.

This also adds a benchmark tool allowing for arbitrary iterations as well as increasing the length of the input data itself.

All existing tests are green, and I've also added several more unit tests. There are now also some test data files (from Redis and Gunicorn) in test/

tmm1 commented 12 years ago

:fire: This looks great. If the simplejson issue is resolved and CI is green, we should branch deploy this to an fe or two and see what happens.

postmodern commented 12 years ago

Has this been tested on Windows?

tmm1 commented 12 years ago

Has this been tested on Windows?

No. Likely will not work, although posix-spawn kinda works on 1.9 in windows.

Either way, it requires python which is not bundled on windows. win32 is not a priority right now. You can continue to use pygments 0.2.x on windows if that works, since it's API compatible.

rtomayko commented 12 years ago

Man this looks awesome. Good shit.

brynary commented 12 years ago

Hey @tmm1, @tnm...

I tried this branch out yesterday, and I think I ran into an issue where Pygments returned the wrong result for a request. Haven't been able to reproduce yet. Is there any chance that a bug in this code might cause Pygments to incorrectly return the result of a previous highlight? (Or perhaps if it's used incorrectly -- maybe somehow something is sharing a socket which shouldn't be, etc.)

Thoughts?

-Bryan

tnm commented 12 years ago

Hey @brynary,

I'm still finishing this up, so I wouldn't use it quite yet. Once the release is done, a bug like that won't be possible :)

-t

brynary commented 12 years ago

@tnm -- Thanks for the heads up. Does the behavior I describe sound like a possible or known bug?

cespare commented 12 years ago

Hey, I'm thinking of writing something like pygments.rb for Go, and I'm curious why you guys switched approaches from the embedded python to a child process. I looked through this ticket and others and couldn't see any mention of the reasoning. Does this approach afford some kind of speedup, maybe? Perhaps using rubypython to embed a python runtime has higher memory usage than a separate process, or can cause instabilities? I'm just guessing here.

tnm commented 12 years ago

Hey @cespare — There were reliability issues with rubypython and the embedded approach; too-frequent segfaults, mostly, as well as difficulty in debugging them. https://github.com/tmm1/pygments.rb/issues/13 was an example that would occur sometimes in tests.

cespare commented 12 years ago

@tnm Thanks, appreciate it.