java.net.Socket implementation #97

bigfatbrowncat · 2013-11-05T22:54:12Z

I created a basic clean-room implementation of java.net.Socket class (and improved some connected additional classes).

The implementation doesn't contain all the methods of the original one, but it is sufficient to connect to a remote server and send/receive data.

I tested it by connecting to an HTTP-server under Windows 7 and OS X 10.8. It works satisfactory.

As for now only TCP-socket is implemented for IPv4 addresses.

If you accept this pull-request I'll try to resolve all possible bugs connected to it. And in addition I'm going to implement ServerSocket soon.

joshuawarner32 · 2013-11-05T23:29:29Z

@bigfatbrowncat , could you take a look at the travis build failures?

https://travis-ci.org/ReadyTalk/avian/builds/13552347

dscho · 2013-11-06T01:02:16Z

classpath/java/net/InetSocketAddress.java

  private final int port;

-  public InetSocketAddress(String host, int port) {


I think that you should not remove that constructor, but replace its body with something like

this(new InetAddress(host), port);

According to Travis, the presence of this constructor is required by this line. It is also in the official Java API...

dscho · 2013-11-06T01:05:47Z

Nice work, overall!

bigfatbrowncat · 2013-11-06T07:29:26Z

Thank you for your attention. I'll fix that.

Strange, but it has builded on my site…

bigfatbrowncat · 2013-11-06T08:23:35Z

I've fixed all the problems, now it builds well.

dscho · 2013-11-06T13:55:39Z

@bigfatbrowncat you probably built with make while Travis builds with make test? Although Traces.java should be built in both cases...

joshuawarner32 · 2013-11-06T17:33:20Z

@bigfatbrowncat, could you squash the existing commits, and then add a test along the lines of SendFile and/or SendServer?

bigfatbrowncat · 2013-11-06T18:03:49Z

Okay, I'll try to squash the commits.

I'm working on ServerSocket now. After that I'll make a test code - send a file from client Socket to a ServerSocket on localhost. Would it be a proper test?

Now it works on OS X Small improvements Added bufferes I/O Small fixes Added getHostName() function

bigfatbrowncat · 2013-11-06T18:23:08Z

I'm sorry for creating a mess, that was the first time I squashed commits.

I will recreate a new pull request with the new branch.

dscho · 2013-11-06T18:50:32Z

classpath/java-net.cpp

+}
+
+extern "C" JNIEXPORT char* JNICALL
+Java_java_net_Socket_allocateBuffer(JNIEnv*, jclass, int buf_size) {


Question is whether that's not better handled with https://github.com/ReadyTalk/avian/blob/master/classpath/sun/misc/Unsafe.java#L14 and https://github.com/ReadyTalk/avian/blob/master/classpath/sun/misc/Unsafe.java#L19?

I made specific methods for using with socket streams. The advantage of implementing everything necessary right here is good encapsulation. We don't call an outer class for a trivial operation. Do you really think we should depend on Unsafe here?

Which disadvantages do you see here (except of duplication of a trivial function)?

I agree with @dscho for a number of reasons:

I'd rather avoid allocating memory not managed by the VM in a non-local fashion

The approach pointed out below (reading directly into the byte buffer) should be more performant (and not require any native allocations)

Java methods can easily be stripped out at build time by proguard, but there's no such tool for jni methods - so I'd like to minimize the latter.

Ok, I will use Unsafe if you insist (you are the boss, after all ;) ).

About reading directly to a buffer - this is not JNI-style, it's Avian-specific. I just haven't known about it yet. Could you tell me more about using it? I could dig by myself, of course, but it would be great if you explain which functions/methods to use and I'll use them.

I understand your position well. I'll try to minimize the native code as tight as I could.

By the way (just an idea), there is method which could help you to shrink native binary. You could divide the default library into some parts (java classes + JNI code) and turn them on and off during compilation. For example if I don't want networking I would exclude the whole java.net package with java-net.cpp.

Of course, the best solution is if you can avoid Unsafe completely ( - but there are lots of competing ideals, and we're unlikely to optimize them all.

(you are the boss, after all)

Technically, I'm just filling in for a while... :)

Could you tell me more about using it?

Sure. Avian supports two types of external functions: Java_ methods (jni) and Avian_ methods (avian-specific, referred to as the "fast" path in the code). Java_ methods should be compatible with every other jvm implementation (if they're not, it's a bug). They take arguments as normal C arguments. Avian_ methods take arguments in a single uintptr_t* array - which allows us to bypass a lot of platform-specific native code. The first argument will be at index 0 in that array, and so on.

The key with Avian_ methods in this context is that there's the unwritten assumption that, since they're already using avian-specific code, they might as well dig further into the internals. That gives us license to use byteArrayBody, an avian-specific utility method for retrieving a reference to an element in a Java-level byte array. Thus, we can ask for the zeroth element and take the address to yield a pointer directly to the buffer.

So you propose to implement send and recv as Avian_ native methods and read/write directly to the buffer?

Ok, I'll try to do that.

…w.google.com

bigfatbrowncat · 2013-11-07T14:29:17Z

Here is the new version.

Native memory allocation for buffer removed and buffer-aware functions changed to Avian_ with the usage of internal API.
Moved the socket API to a different file socket.cpp which can be used from different classes (for example, I'm going to make Socket.init() private in order to hide it from the user, so there should be a universal Socket API for, e.g., java.nio.
Added a unit-test which simply requests www.google.com/. Unfortunately, there is no ServerSocket there (I'm going to add it), so we can't just connect one socket to another and check if data transferred correctly. But you could check that the response from Google site is correct.

Do you have any other comments?

joshuawarner32 · 2013-11-07T15:23:18Z

So... I mentioned a " test along the lines of SendFile and/or SendServer" without actually having read over that code. Oops.

In particular, they live in the test/extra directory and don't actually get executed with the other tests. They must be manually executed. On the other hand, they also don't make any requests out of the local machine (well, unless you tell them to on the command line) - which I think is important for unit tests (even though they're not really unit tests).

If possible, I think the best test would be one that spins up an extra thread and sends a short string between that thread and the main one via your new socket implementation.

Also, see my comment(s) on the travis build failures.

bigfatbrowncat · 2013-11-08T00:25:18Z

I have a problem. I tried to build this code under OS X (yes, you were right, I had to write reinterpret_cast), but it wouldn't compile.

The short story: I can't use "jni.h" and "avian/machine.h" included in the same file. They are conflicting.

But I need both. If I don't include the second, I can't use vm::byteArrayBody function. If I don't include the first, I can't call e->GetStringUTFChars(…) - there is just no such function in class Thread - it should be cast to JNIEnv in order to use this function.

I have some hacking ideas about how to make them friends, but I want to know the "official" way. Could you propose me how should I do?

dscho · 2013-11-08T00:59:56Z

@bigfatbrowncat are you sure you need to call GetStringUTFChars()? If you have a look here you will see how I implemented a native method accepting a String on the Java side and operating on an object using the stringLength() and stringChars() functions on the C++ side. (Hint: look for the native nextResourceURLPrefix() method.)

joshuawarner32 · 2013-11-08T03:47:11Z

@bigfatbrowncat , I agree with @dscho that stringLength() and stringChars() is probably the way to go - but on a whim, I removed the incompatibility between jni.h and machine.h that you noted in 3c1afdd.

bigfatbrowncat · 2013-11-08T07:16:20Z

I agree too (about stringLength() and stringChars()), but I think that changing it along with my modifications is too much for now. I haven't written the method which uses it and I don't want to break it accidentally. It works now. Let me leave it as it is, please.

I merged all the latest commits into my fork and fixed bugs. Could you look at the most recent version, please?

bigfatbrowncat · 2013-11-08T07:23:36Z

Hmm… strange build error. I don't have such a bug under OS X. I'll check it under Win7...

bigfatbrowncat · 2013-11-08T07:57:59Z

I found this bug in Win7 and fixed it. It's just a piece of C linker magic. avian/machine.h should be included before jni.h.

bigfatbrowncat · 2013-11-08T08:04:24Z

Hmm... now it builds under Win7, but still doesn't build in Travis. I don't have a linux machine here. Could you check?

joshuawarner32 · 2013-11-08T15:38:22Z

@bigfatbrowncat I removed another incompatibility between jni.h and avian's common.h in 2800ffe. That seems to fix the linux build. Could you pull and verify on your end?

bigfatbrowncat · 2013-11-08T16:14:52Z

It builds on OS X and under Win7 x64. And all the tests work like a charm. Finally!

joshuawarner32 · 2013-11-08T17:09:05Z

I've squashed the commits in this PR and merged them: 45ee25f
I've also modified the Sockets test a little: c3bbe55

Thanks, @bigfatbrowncat !

dscho reviewed Nov 6, 2013
View reviewed changes

This version simply works on Windows

b71aca0

Now it works on OS X Small improvements Added bufferes I/O Small fixes Added getHostName() function

bigfatbrowncat closed this Nov 6, 2013

bigfatbrowncat deleted the sockets branch November 6, 2013 18:17

bigfatbrowncat restored the sockets branch November 6, 2013 18:18

bigfatbrowncat deleted the sockets branch November 6, 2013 18:22

bigfatbrowncat reopened this Nov 6, 2013

dscho reviewed Nov 6, 2013
View reviewed changes

Changed sockets API to Avian-style, added unit-test which requests ww…

baa4786

…w.google.com

bigfatbrowncat added 2 commits November 8, 2013 10:39

Merge branch 'master' of github.com:ReadyTalk/avian into sockets

62ba493

Fixed namespaces, indentation and SD_* constants under unix

2f1482c

build fix

65ace29

bigfatbrowncat added 2 commits November 8, 2013 16:27

Trying to workaround Travis build error

a408917

Another try to fix build

868b9b5

Merge branch 'master' of github.com:ReadyTalk/avian into sockets

36948cc

joshuawarner32 closed this Nov 8, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

java.net.Socket implementation #97

java.net.Socket implementation #97

bigfatbrowncat commented Nov 5, 2013

joshuawarner32 commented Nov 5, 2013

dscho Nov 6, 2013

bigfatbrowncat Nov 6, 2013

dscho commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

dscho commented Nov 6, 2013

joshuawarner32 commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

dscho Nov 6, 2013

bigfatbrowncat Nov 6, 2013

joshuawarner32 Nov 6, 2013

bigfatbrowncat Nov 6, 2013

joshuawarner32 Nov 6, 2013

bigfatbrowncat Nov 6, 2013

bigfatbrowncat commented Nov 7, 2013

joshuawarner32 commented Nov 7, 2013

bigfatbrowncat commented Nov 8, 2013

dscho commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013

		private final int port;

		public InetSocketAddress(String host, int port) {

java.net.Socket implementation #97

java.net.Socket implementation #97

Conversation

bigfatbrowncat commented Nov 5, 2013

joshuawarner32 commented Nov 5, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dscho commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

dscho commented Nov 6, 2013

joshuawarner32 commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

bigfatbrowncat commented Nov 6, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bigfatbrowncat commented Nov 7, 2013

joshuawarner32 commented Nov 7, 2013

bigfatbrowncat commented Nov 8, 2013

dscho commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013

bigfatbrowncat commented Nov 8, 2013

joshuawarner32 commented Nov 8, 2013