Remove protective copies in Thrift APIs

[ctubbsii]

Generate thrift with unsafe_binaries flag to remove unwanted protective copies of ByteBuffer types (we will copy as needed ourselves).

[ctubbsii]

There are a few other options we may want to look at for performance, too. In particular, the option_type=jdk8 (which I intend to do in a subsequent PR), the future_iface (which may be interesting to use), and the reuse_objects (which might require turning back on the protective copies in the APIs) options.

thrift -help 2>&1 | sed -n '/^\s\sjava\s/,/^\s\s\w/p'

[dlmarion]

These changes look good

[DomGarguilo]

LGTM as long as it addresses the issue outlined in #6455

[dlmarion]

So, these changes look good, but there is still an issue. Take this code for example:

accumulo/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftScanner.java

Lines 970 to 972 in 618addf

	for (TKeyValue tkv : sr.getResults()) {
	results.add(new KeyValue(new Key(tkv.getKey()), tkv.getValue()));
	}

The code before #6430 looked like:

accumulo/core/src/main/java/org/apache/accumulo/core/clientImpl/ThriftScanner.java

Lines 967 to 969 in 5f16282

	for (TKeyValue tkv : sr.results) {
	results.add(new KeyValue(new Key(tkv.key), tkv.value));
	}

The code in #6430 made the members private, so when accessing TKeyValue.value the calling code had to call either TKeyValue.getValue() or TKeyValue.bufferForValue() as below.

accumulo/core/src/main/thrift-gen-java/org/apache/accumulo/core/dataImpl/thrift/TKeyValue.java

Lines 177 to 184 in 618addf

	public byte[] getValue() {
	setValue(org.apache.thrift.TBaseHelper.rightSize(value));
	return value == null ? null : value.array();
	}
	public java.nio.ByteBuffer bufferForValue() {
	return org.apache.thrift.TBaseHelper.copyBinary(value);
	}

The changes in this PR only removes the defensive copy from bufferForValue, so the calling code needs to change as well.

[dlmarion]

Either the code calling the Thrift objects needs to be checked / corrected, or maybe some other Thrift compilation flag needs to be used so that copies are not made in the getX methods.

[ctubbsii]

I'm abandoning this change. I don't think it's a good idea anymore for the following reasons:

1. This also removes the protective copies from the setters/constructors, which existed prior to Use private Thrift members #6430 and we didn't have a problem with those. I don't think it's worth removing those along with the ones for the getter.
2. The .getValue() method that returns a byte[] does not change with this PR, and that's the one that we are now using, not the .bufferForValue() method, which is affected by this change. This means that our code isn't really impacted by this change. The bigger impact from Use private Thrift members #6430 is just the switch from direct access to the internal ByteBuffer to the method that returns a byte array, rather than one that makes a protective copy of the ByteBuffer. The equivalent to before this PR would be to use the .bufferForValue() version of methods, but I don't necessarily think that's a good idea either.
3. The method we are using, .getValue(), does not actually make copies, except in the case where a copy is needed to truncate the underlying byte buffer's array to size. Other than that, there's just a very lightweight object allocation to wrap with a ByteBuffer interface, which I suspect is not really the cause of any performance issues.
4. I think ultimately, any issues with the RPC layer are really best addressed in calling code, but I think the protective copies and the use of
5. It should be noted that the GarbageCollector IT that started failing after Use private Thrift members #6430 is already flaky, and sometimes times out even after reverting Use private Thrift members #6430. While Use private Thrift members #6430 may have made the situation slightly more reliably fail, I still think that the best fixes are probably in the calling code, and that test probably needs to tuning anyway and should not be considered a reliable indicator that the RPC changes are a problem overall.

We can continue the discussion on #6455 for possible next steps.