systemed
commented
2 months ago Thank you!
I came to a similar conclusion (except with even more "wouldn't understand what they're referring to"). I think merging this now makes a lot of sense and then we can try and optimise the multiple-pbf scenario if that's needed - it's a bit of an edge case anyway.
(Incidentally, life has got in the way recently so I haven't been able to spend as much time on tilemaker as I'd like - sorry! But I'm hoping to clear the backlog asap and then we can put a new release out.)
cldellow
See issue https://github.com/systemed/tilemaker/issues/747.
The C++ powers that be have realized that something is redundant. The Boost people have removed this redundancy, and the guidance is to use the Other Way of Doing The Thing.
The only problem is... I don't know what the Other Way is. From context I infer there's some static method we ought to be able to invoke, and it's so obvious that someone who writes custom allocators would just immediately understand what they're referring to. That's not me, though.
But...do we even care? Can we just... not call the destroy function?
I think destroy only gets called when deallocate is called. I think we only call deallocate in the case when a user reads multiple PBFs. I think that's an uncommon case. I think (big grain of salt - I skimmed, and I'm out of my depth here) that boost will free up the underlying memory if all of the objects in a given segment are deallocated. So probably, in the multiple PBF case, we introduce a memory leak by not calling the destroy function.
That's not great, but it's probably fine? If you're processing a bunch of small PBFs, you'll probably still be able to do so. If you're processing large PBFs, you'll still be saved by
--store.This is, obviously, an incredibly hacky fix for this issue. But I wanted to send some PRs for other bugs, and I want to get the build to work first. :)