Context local storage SPI

[vietj]

An SPI for interacting with local context storage.

The existing implementation uses on a concurrent hash map guarded by the context instance.

This new implementation relies on prior knowledge of the set of context locals, so the context allocates upfront the required space to hold the local storage, reducing synchronisation and simplifying lookup that uses allocated index instead of hashed string lookups.

The existing local context data is retrofit as a context local.

[vietj]

cc @tsegismont

[vietj]

cc @cescoffier

[tsegismont]

Good job @vietj !

[vietj]

See that rather as a compute operation.

…

On Fri, Dec 22, 2023 at 2:11 PM Francesco Nigro ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/main/java/io/vertx/core/impl/ContextBase.java <#5048 (comment)> : > + ContextKeyImpl<T> internalKey = (ContextKeyImpl<T>) key; + int index = internalKey.index; + if (index >= locals.length) { + throw new IllegalArgumentException(); + } + Object res; + while (true) { + res = LOCALS_UPDATER.getVolatile(locals, index); + if (res != null) { + break; + } + Object initial = supplier.get(); + if (initial == null) { + throw new IllegalStateException(); + } + if (LOCALS_UPDATER.compareAndSet(locals, index, null, initial)) { We don't need atomicity here, but the same semantic of putLocal, which doesn't care of races, imo. If we need that level of atomicity we can have a new method which ensure an atomic change and use the compare and set. — Reply to this email directly, view it on GitHub <#5048 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABXDCRV5EQ7ZV52SCECO7LYKWBIZAVCNFSM6AAAAABA64S6DKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJUGQ3DEMBRG4> . You are receiving this because you were assigned.Message ID: ***@***.***>

[cescoffier]

In Quarkus we don't know the key that may be used. We can imagine knowing a few of them at build time, but we will have an unknown set at runtime.

I would do something like:

• at build time, request extension to give the keys
• at runtime, allocate with this set of keys +1
• the +1 is a concurrent hash map for all keys that have not being declared.

WDYT?

[franz1981]

It is a nice approach @cescoffier which will give performance and flexibility based on the usage, I like it!

[vietj]

@cescoffier note you can have your extension that uses a map (or whatever structure you need) as a slot with a unique key, giving you opportunity to proceed as you want, which is what reactiverse context logging will do, e.g. it could be a ContextKey<Map> and the Map would be managed by Quarkus.

[franz1981]

I have some mixed feeling after re-reading what @cescoffier rightly said, and remind me about something I've worked recently eg quarkusio/quarkus#38761 which has a very similar problem ie at build time it can collect what are the "supposely" used keys (in the PR I've referenced are bean ids, but is the same really...), ie: what if the number of actually used keys is much smaller than the ones preallocated?

In shorts, constant lookup access and simple storage (no keys, just values) is traded against the cost of overestimating the capacity, which is not good here. In quarkusio/quarkus#38761 I've tried to keep the allocated cost at it(s) minimum by having lazily allocated parts and I think that we should do something similar for this one, here as well.

Furthermore, I suggest to add some microbenchmarks (I can help with it @vietj ) to compare this approach as it is, but using different key used/total key generated ratios vs a "normal" (but smarter) concurrent hash map, because, for small utilization ratios (<= 1/10) the cost of allocating a full blown array vs using a CHM could be not that different.