jjwt
jjwt copied to clipboard
jwtk
Microoptimalization: Dates ++
Hi,
I'm looking into the performance of the decoder, and though I'd start with at few low-hanging fruits.
Reduce the (unnessesary) use of throwaway Date objects.
Remove allowSkew variable.
Use a big enough StringBuilder.
Hi @skjolber ! Thanks very much for this - how much of a change in performance does using millis instead of Date make?
I ask because most people have asked for JDK 8 DateTime support as the default way of representing timestamps, and that's on the horizon (see #308 and other linked issues). If that's the case, does this change still make sense?
@lhazlewood there is not much of a difference in performance by it self, but gotta start somewhere; doing unecessary stuff on the critical path should be avoided.
I guess creating Instant instead of Date is not much better. I think long as the time format is the best and most simple, at least as an internal representation. Seems in the time PRs Instant just replaces Date, so the same fix just with Instant would be ok.
Or even better: Create a comparison method for date fields, perhaps just for internal use, so that the claims value can be compared directly, i.e. with as little overhead as possible. So in other words, I'd pass in parameters "claim name" + "limit" and the new method throws an exception if the claim is not as expected.
All this does is save 1 object instantiation (The constructor for date just calls and stores System.currentTimeMillis()). Seems kinda unnecessary.
Just a note - we'll probably need to hold off on any changes like this until we address #311, at which point the Clock interface (and others) may change. To minimize the amount of forced implementation requirements on a version upgrade (or across multiple upgrades), I'd like to address this all at once.
@DanFTRX well something is better than nothing, and this PR also simplifies the logic a bit, as a bonus. But I think if I wrote this again, I'd simply extend the claims logic; adding support for time comparisons, so that no object creation needs to happen at all.
@lhazlewood fair enough.