[SQL]RowBasedKeyValueBatch reuse valueRow too

[yaooqinn]

What changes were proposed in this pull request?

reuse the cached valueRow in RowBasedKeyValueBatch

How was this patch tested?

existing ut

[yaooqinn]

cc @ooq@sameeragarwal@davies@srowen is it right and necessary?

[AmplabJenkins]

Can one of the admins verify this patch?

[ooq]

Thanks for the patch @yaooqinn . As per comments for getValueRow, because getValueRow(id) is always called after getKeyRow(id) with the same id, we use `getValueFromKey(id) to retrieve value row and use the id as a flag for cached/uncached. The patch is unnecessary IMO. Also, with the patch code, it seems value row is not pointing to the correct position?

[yaooqinn]

@ooq Thanks very much for your review. As we always getKeyRow(id) before we getValueRow(id) with the same id, we don't retreive the keyRow from the batch but use the cached one. IMO, we can mark the valueRow as cached too because no action seems to overwritten it, then we'd rather use the cached valueRow than retrieve a new one .

[ooq]

@yaooqinn Do you have any benchmarks on the performance difference? I think pointTo() is pretty cheap. And does the patch pass the tests? I think valueRow is not updated correctly in your patch.