Optimise SnodeAPI and Add Snode.Version and tests

[bemusementpark]

This PR was motivated by recent ANRs cause by a Snode#version comparison in an inner loop. This PR moves the heavy lifting of parsing a version to the construction time of the Snode object. It also caches them in a Map to eliminate practically all calculations.

This PR introduces value class Snode.Version and many simplifications and optimisations in SnodeAPI

value class Snode.Version should be a zero-cost abstraction on a Long representation of a Snode's Version, an optimisation on the current storage of String, and introduces type-safety. It accomodates being created from a String of 4 . separated shorts.

Accepted formats include:

major
major.minor
major.minor.patch
major.minor.patch.patch

where each part is < 65536, which is... conservative. You'll see in the Test.

Is it optimised? Yeah, it should be...

for example, to construct from a String, I've got:

internal constructor(value: String): this(
            value.splitToSequence(".")
                .map { it.toULongOrNull() ?: 0UL }
                .foldToVersionAsULong()
        )

which allocates a String for each part, but it might be necessary, as we have to parse some String to some Int anyway, so we might be getting quite agricultural if we try to optimise to avoid allocating Strings.

Anyway, as most nodes will be running the latest 2-3 versions, I've added a cache which should remove practically all calls to the above constructor.

private val CACHE = mutableMapOf<String, Version>()
@SuppressLint("NotConstructor")
fun Version(value: String) = CACHE.getOrElse(value) {
    Snode.Version(value)
}

I'm even keeping the List<Int>#joinToString(".") which can be optimised, because I expect the versions to be uniform and this cache to be much more effective than leveraging their integer type.

As this Version is a nested class of Snode (could make it top-level 🤷 ) I've added the cached courtesy-non-constructors to a companion object in Snode. This gives them the same imports as the actual constructors.

FAQ:

Is this just changing the code for no reason?

No, it's good, there's lots of good simplifications and optimisations.

in SnodeAPI

0. LokiAPIDatabase.kt was creating a Snode and null checking everything in 3 different places, so there was a lot of deduplication there.
1. seedNodePool was lazy for no reason.
2. Visibility was unnecessarily broad, giving irrelevant code-completion tips.
3. consts were not labelled as such, preventing optimisations.
4. ThreadUtils#queue was used with Promise#deferred in a complicated manner that served no purpose. I simplified this to task {}s which will more trivially be converted to coroutines.
5. mapOf syntax takes Pairs which are unnecessary allocations, I swapped it to buildMap as some of this code is called a lot, and even inner-loop.
6. getRandomSnode always used the same params, I extracted it to a constant.
7. one signing and encoding pattern was repeated 10 times, I extracted it to a few functions.
8. There was quite a few intermediate strings being constructed unnecessarily, especially for signatures, so I've optimised that to use Sequence<String> and create a ByteArrays from them directly.

e.g.

val message = (userPublicKey + serverHashes.fold("") { a, v -> a + v } + hashes.fold("") { a, v -> a + v }).toByteArray()
val message = sequenceOf(userPublicKey).plus(serverHashes).plus(hashes).toByteArray()

[simophin]

LGTM

[ThomasSession]

I get that the current receiving functions of getAccountID are in try/catch but I'm not sure which should be mixing "philosophies" here. If this is still using the ol' Promises, then it should probably be handled with that mechanism instead of throwing

[bemusementpark]

interface Promise<out V, out E> has many implementations. In this part I've swapped a deferred for a task.

With a Deferred you call resolve or reject

In task(..., body: () -> V) you return in the lambda or throw

task has narrower more clearly defined scope than deferred. Using deferred implies that we will pass it somewhere else, but this does not occur, task is clearer.

[ThomasSession]

where is task created there though?

[bemusementpark]

later, yeah, that's an issue... I could return Promise.ofFailure here, or would have to make this whole function a task and then bind to the next promises.all thing.

The former sounds better.

[bemusementpark]

This is a really good one to do with coroutines, but maybe later...

fixed, using task {}s because there's all this work at the start.

I wanted to

task {
    ...
} bind {
    all(promises) ...
}

but there's too much stuff in the bind that need stuff from the task, so I've gone:

task {
    ...
    if (...) throw Exception
    ...
    task {
        ...
    }
}.unwrap()

because there's no function like the RxJava deferred... that would let us return the other task "flatMap" style.

And there's not much point writing something like that, when we want to remove Promises entirely.

[ThomasSession]

It seems you have inverted the logic here by checking containsAll instead of !=

[bemusementpark]

fixed! 🎉