Deprecate the Text class

[hrosa]

The SPI module contains an in-house replacement of String, called Text. This class is pointed by the profilers as one of the biggest trash source in the project. It is heavily used by the MGCP stack.

[anthonyms]

Kindly provide more info on this as I am inclined to have a go at it.

Would a simple replace with String suffice?

[hrosa]

Hi,

Unfortunately it will not be so simple, otherwise we would have implemented our stack with String from the beginning :)

Using the String class provided by JDK may result in some poor performance, especially on heavy text-based modules like MGCP and SDP. For that reason, the Text class was created.
Unfortunately, after some profiling we discovered that this Text class is keeping some trash in memory, holds duplicate contents, initializes arrays with null elements, etc etc. And its interface is a bit clunky.

So, we've decided to deprecate this class and replace it with a well-proven library, like Javolution (for example). This library offers a Text class that is proven to perform well.

As you see, this task falls in the optimization area and requires a bit of experimentation.
I would advise you to:

1. Perform load test on the current master branch and profile the Text class (used mainly in the MGCP module).
2. Create a new branch and replace current Text class with Javolution's Text.
3. Repeat the tests you did on (1) and verify if there were some improvements.
4. IF Javolution's Text helps increase performance, then apply it to SDP module (currently String-based).
5. Repeat tests.

We really appreciate that you reach out to us, we truly welcome and encourage contributions from community.
If you decide to contribute to this task, then feel free to reach out to us in our public forum, where both community and core team can help you. You are not alone :)

In case you don't feel prepared to take on this specific task, we have other tasks that you can help with.

Best Regards,

• H

[anthonyms]

Ill look at it some more and get back to you.

[hrosa]

@artfullyContrived made up your mind? :)

[anthonyms]

It is not a simple drop-in replacement as I had thought it is:

1. The Javolution Text class is final thus can't be sub-classed. A quick
   work-around would be to have a Wrapper class. Inability to subclass makes a
   few things hard. e.g Current Text class comparison is case insensitive.
   Current Text is also multiline.
2. A few methods available on the current Text class are not available
   on the Javolution class. e.g split, divide, Constructor(byte[], int, int).

This makes using javolution Text class as a direct replacement non trivial.

If you could advice on how to proceed I'd love to continue on this one.

On Tue, Feb 9, 2016 at 3:07 PM, Henrique Rosa notifications@github.com
wrote:

@artfullyContrived https://github.com/artfullyContrived made up your
mind? :)

—
Reply to this email directly or view it on GitHub
#57 (comment)
.

[hrosa]

@artfullyContrived Sorry for late response, I am currently in Japan attending some events.

Right, there is no easy way around this one I'm afraid.

So, basically there are two modules that are heavily text-based: SDP and MGCP.
A possible approach to this task is:

1. Identify a specific part of the code that need to be replaced and do it iteratively.
   Example: Inside MGCP module you have classes to handle CRCX, MDCX, DLCX requests.
2. Write unit tests to make sure text output is the same after replacing with Javolution class.
3. Run load tests to measure performance gain.
   You can first run test on master branch (original version) and then on your branch for comparison.

Even though you cannot extend Javolution Text, you can write a TextUtils class where you implement necessary operations on top of Javolution.

As you see, it's actually a big task and not a simple class replacement.
But I do promise that by the end of it, you will have learned a lot about the project :)

[anthonyms]

Noted.

I will get back to it later this week.