Repository navigation
Make PseudoJet and EEJet isbits structs #145
Description
Coming from #88, passing mutable structs between C and Julia interfaces requires extra copies and allocations since they are differently stored in memory. This happen very often as the API consists of multiple smaller function passing around the ClusterSequences and jet objects. Unfortunately I don't have a concrete estimate on a cost of it.
Changing the PseudoJet and EEJet to be immutable structs and isbits would allow one to remove that extra overhead.
I think it should also have a potential to improve slightly the performance on Julia side since then more continuous blocks of memory could be used.
Hopefully this should be possible with a moderate effort since the only moment when mutation is required seems to be adding history information
Activity
Moelf commentedon Mar 29, 2025
does making them
isbitssolve this? it's possible for them to have some kind of padding difference? just asking, im not expert in thism-fila commentedon Mar 29, 2025
Right some more details could be useful.
isbitsare guaranteed to be stored inline (and have a corresponding C-type with the same sizeof/field layout/padding etc) while the rest are stored as a pointer to an actual object. This really shows up for vectors (which are very prominent in JetReconstruction public API):Vector{JetButIsbits}is a continuous memory block and each element is an object which is the same asJetButIsbits[]in C, also pointer and length is good representation for both. It makes it very fast to work with as conversions/copying can be done on a whole memory block, eg.memcpyif copy is needed,unsafe_wrap.Vector{MutableJet}is a continuous memory block which elements point to actual objects allocated somewhere on heap (each separately, also how it's done exactly is internal to Julia). Due to this extra indirection C and Julia can't directly share a common memory block and making conversions/copy is done for each element separately withunsafe_load/unsafe_storem-fila commentedon Mar 31, 2025
This also probably was missing from the original comment.
EEJetandPseudoJetcan be madeisbitsby changing them to immutablestructas they fulfill other requirements alreadygraeme-a-stewart commentedon Mar 31, 2025
I looked at this issue a while ago, to see if it would improve performance and the change was not too hard - a slightly less "natural" code path, nothing awful. However, the performance was only improved by a tiny amount (~0.5% comes to mind), so I didn't think it was worth it.
However, with now some significant benefits for #88 and #138 it now looks worthwhile doing.
@m-fila - do you want to take this one on?
m-fila commentedon Mar 31, 2025
Sure I can take it, but I'll probably only start working on it in about two weeks
btw I'm not sure if this is "internals", more like "interface change" to me since users see
PseudoJetandEEJetquite often 😉graeme-a-stewart commentedon Apr 1, 2025
I'll assign us both and we see who gets to it first... While I take your point about an interface change, mutability for the user was more of an implementation leak than a feature. Anyway, this will be for 0.5.0 where we are allowed to break things 😁
Moelf commentedon Apr 1, 2025
for posterity, working with immutable probably can greately benefit from https://github.com/JuliaObjects/Accessors.jl
graeme-a-stewart commentedon Apr 9, 2025
FYI, I started to work on this @m-fila