Question about the Java code

[aozarov]

I am reading the ice code related to the network operations and have 2the following questions.

1. BasicStream.java -> I see that you commented out the allocateDirect and use heap buffer instead. Any reason? Our tests shows much better performance for the DirectBuffer as long as it is re-used.

2. ThreadPool#read(EventHandler handler) -> After reading the payload size (int size = stream.readInt()). it looks like you are reading from the socket with expectation it will have that content/playload readily available (making the bytebuffer its size and reading with zero timeout). Do I read it correctly and if so how can you guaranty that (the palyload will be available to a non-blocking read)?

Thanks,
Arie.

[benoit]

Hi,

Originally Posted by aozarov

I am reading the ice code related to the network operations and have 2the following questions.

1. BasicStream.java -> I see that you commented out the allocateDirect and use heap buffer instead. Any reason? Our tests shows much better performance for the DirectBuffer as long as it is re-used.

I believe we have experienced problems with direct buffers in the past and decided to disable them. Can you detail the tests where you've seen it perform better? Which Ice, OS & JDK version are you using?

2. ThreadPool#read(EventHandler handler) -> After reading the payload size (int size = stream.readInt()). it looks like you are reading from the socket with expectation it will have that content/playload readily available (making the bytebuffer its size and reading with zero timeout). Do I read it correctly and if so how can you guaranty that (the palyload will be available to a non-blocking read)?

No, we don't expect all the data to be available. The read might not complete immediately if not enough data is available on the socket and if that's the case the thread returns to wait on the selector for more data to be available.

Cheers,
Benoit.

[aozarov]

The DirectBuffer vs non direct/heap buffer were done independently of ice.
The platform was Linux (Suse 10) and JVM is Sun JDK 1.6.

But I see this line:
moreData = handler.read(stream);
assert(stream.pos() == stream.size());

Doesn't it suggest you are expecting to fill the buffer (which was re-allocated to the size of the payload)?

[benoit]

Hi,

Originally Posted by aozarov

The DirectBuffer vs non direct/heap buffer were done independently of ice.
The platform was Linux (Suse 10) and JVM is Sun JDK 1.6.

We'll check whether or not we can enable again direct buffers.

But I see this line:
moreData = handler.read(stream);
assert(stream.pos() == stream.size());

Doesn't it suggest you are expecting to fill the buffer (which was re-allocated to the size of the payload)?

Note that discussing the Ice internals is a bit out of the scope of the free support we can provide on the forums. But to answer your question, the read() call throws Ice::TimeoutException if the read can't complete without blocking so this code is fine (with the upcoming Ice 3.3.0, this code changed a bit, read() doesn't throw anymore in this case).

Cheers,
Benoit.

[aozarov]

Hi Benoit,

Thanks for your answer. I understand if you can't shed more light about ice internals for free or if you are convinced that there is no bug here.

In case we can continue this discussion for a little longer, I am still not sure about the current behavior. The TcpTransceiver#read will loop until the buffer is full (and all the message payload was received), socket was closed or no more data has arrived from the last non-locking read call. In this case timeout is set to zero and therefore we should not enter the blocking read section. As there is no guaranty that all the payload will be available without blocking (or waiting for it) than we can, as you said, get Ice::TimeoutException. That means that you are expecting for the most part for the payload to be fully available (otherwise you will get a high rate of message failures - as this part is not controlled by a time constraint), no?

Thanks,
Arie.

[benoit]

Hi Arie,

No, if you check out the thread pool code, you'll see that the Ice::TimeoutException exception is catch explicitly and that this exception is just a way to indicate to the thread pool that the read couldn't complete without blocking and that it should go back waiting on the selector for more data to be available -- it does not indicate a failure. The thread pool does not expect all the data to be available in a single read() call: a message can be read in many successive non-blocking read() calls.

If you're still not sure how this works, I recommend checking out the source code of Ice 3.3b instead. We've simplified this code a bit to not use anymore an exception to indicate that the read couldn't complete without blocking.

Cheers,
Benoit.

[aozarov]

Hi Benoit,

Thanks! yes I see it now. I didn't trace it further as I didn't imagine exception would be used as a standard flow control. Great to hear that it was changed.
BTW, was the ConnectionI#parseMessage moved out from the synchronized (or any plans for it).

[benoit]

Hi,

No, parseMessage wasn't moved out of the synchronization, it needs to be synchronized since it access mutable attributes of the connection. The only thing that might be useful to move out of the synchronization is the Bzip2 un-compression and I suppose that's why you're asking about it . We'll discuss if this can be changed.

Cheers,
Benoit.