Comments (10)
@jvican see mihaip/nailgun@a738fd6 for the code and a simple benchmark. If you run it, you'll see:
$ python -m pynailgun.test_ng TestNailgunConnection.test_echo_latency
NailgunConnection: 965.1ms (min: 507ms, max: 1058.3ms)
BytesInputNailgunConnection: 4.2ms (min: 2.7ms, max: 6.8ms)
.
----------------------------------------------------------------------
Ran 1 test in 9.888s
OK
NailgunConnection: 962.9ms (min: 503.2ms, max: 1062.6ms)
BytesInputNailgunConnection: 3.9ms (min: 2.5ms, max: 8.6ms)
.
----------------------------------------------------------------------
Ran 1 test in 9.880s
OK
from nailgun.
not on master yet, hopefully will go out this week
from nailgun.
I suspect that this has something to do with the fact that both the C client and Python client (and thus probably also the Java server) have diverged from the spec and now require a "start reading stdin" chunk after every stdin chunk is sent from the client. The constant ping-ponging certainly doesn't help performance.
from nailgun.
That's been my observation too. I was seeing response times with a bimodal distribution of either 1000 or 500ms, even though the actual work on the Java side took ~30ms.
The problem was that the Python Nailgun client reads stdin in a separate thread, which races with the main thread
1 [main thread]: _process_next_chunk calls select()
2 [stdin thread]: wait on ready_to_send_condition
3 [java process]: sends CHUNKTYPE_SENDINPUT
4 [main thread]: select() returns, process_nailgun_stream is called, which notifies ready_to_send_condition once it determines that CHUNKTYPE_SENDINPUT was sent
5 [main thread]: calls _check_stdin_queue, but the stdin thread hasn't finished reading stdin, so there's nothing there
6 [stdin thread]: ready_to_send_condition is done waiting, stdin is read, data is added to the stdin_queue
7 [main thread]: next iteration of _send_command_and_read_response, select() is called again. The java process is still waiting to read from stdin, so select() blocks until the timeout fires (normally 500ms)
8 [main thread]: _check_stdin_queue finally has data, so the stdin can be sent to the java process
Steps 5 and 6 happen concurrently, but because we don't start reading stdin until after the condition notification happens, we most likely will not enqueue anything before the check happns.
In our case, we knew exactly what we want to send to the java process, we shouldn't need to go through a reader at all. Therefore we introduced a simpler BytesInputNailgunConnection
that directly sends a bytes
instance to the java process without needing a separate thread at all.
from nailgun.
@mihaip Can you share more about the BytesInputNailgunConnection
? I'm also experiencing this issue downstream and it's quite annoying, since I was using stdin/stdout for LSP.
from nailgun.
cc @sbalabanov
from nailgun.
This should be fixed with Nailgun Python client 0.9.3
from nailgun.
@sbalabanov is that version on master (or otherwise publicly accessible -- there are no recent commits in https://github.com/facebook/nailgun/commits/master/pynailgun)? I'm not seeing any performance difference in my benchmark.
from nailgun.
PR is merged now. I'll try it out downstream to see if it works for me. @sbalabanov I assume this has already been integrated with buck?
from nailgun.
Yes it is integrated with buck but not a part of any official release yet
from nailgun.
Related Issues (20)
- How to properly terminate the nail? HOT 2
- Overhead of SecurityManager HOT 17
- Publish nailgun to maven central HOT 2
- ng.py crashes in a cygwin environment when trying to reference Kernel32 dll
- Problem with nested connections to a unix domain socket HOT 2
- Publish 0.9.3 artifacts to a public repository HOT 2
- Nailgun server crashes HOT 8
- NGServer or ng client changes path delimiters on output HOT 2
- Documentation is lacking HOT 2
- Running `mvn package` fails with JavaDoc errors HOT 1
- Support Java versions greater than 8?
- Run nailgun without classpath as code comes on the fly HOT 1
- JDK 11/12 Support HOT 2
- Remove / tone down the scary 'NOT MAINTAINED' header
- Server's sockets are not shut down in an orderly fashion; sending heartbeats can occasionally fail HOT 1
- Release? HOT 1
- Unused value
- Make error HOT 1
- still mantained? HOT 2
- Mirroring policy breaks repositories HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from nailgun.