I have optimized the code

[sandeepkalra]

New Code uses strings builder to do at least 50% faster processing.

[ocram]

Thanks!

Could you perhaps just add the chars in the reverse order again, instead of requiring a whole additional function reverseChars?

Moreover, what’s reverseRunes? It doesn’t seem to be used at all.

And, again, are the two Codec blocks necessary?

[sandeepkalra]

The string builder and byte builder do NOT give access to the internal array to push in reverse order. So, I have to reverse it after the string is built from it. In my benchmarking, I found that this may be taking 30-40% of total CPU when it comes to Encode(). I will continue to search for better options, but overall this solution is 55% faster than the original where I was using "string" in for loop.

[sandeepkalra]

I have removed reverseRunes. The idea was to say that if they go and put Unicode in Alphabet list, then they have to switch to reverseRunes()

I have made the 2 functions independent of codec block.

Also, I did study strings. builder writers and byte array writers, but they do not give control to write in reverse into internal array.

Best Regards, Sandeep Kalra

On Thu, Jun 14, 2018 at 2:56 PM Marco [email protected] wrote:

Thanks!

Could you perhaps just add the chars in the reverse order again, instead of requiring a whole additional function reverseChars?

Moreover, what’s reverseRunes? It doesn’t seem to be used at all.

And, again, are the two Codec blocks necessary?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/delight-im/ShortURL/pull/32#issuecomment-397419053, or mute the thread https://github.com/notifications/unsubscribe-auth/AFTvwpstRQrSdw-X12o0bJBY5X3RnQ5wks5t8r_jgaJpZM4UoXVV .

[bgadrian]

To write in reverse I think the only way is to use a normal slice of Runes, based on the specific usage of the function (we have a limited, known set of runes) we should not have any problem (ones that are fixed by bytes.buffer and strings.builder). I think if the performance gained is significant it worth the change (from builder to slice).

The Grow formula is the same (2*n), for the builder internal slice and for a regular slice, so memory allocation should no differ, but we will eliminate 3 casts (string, bytes, string) and n/2 swap operations (from the reverse).

[bgadrian]

As for the test you can use 2 other implementations with some random numbers, and use those as a test in the Go version. Also you can add the examples from the Readme and calculate the first values by hand. 3141592 <=> vJST 123456789 <=> pgK8p

I am surprised that no other implementation has tests, so Go must be better.