rdblue commented on PR #18544: URL: https://github.com/apache/iceberg/pull/18544#issuecomment-6350991252
> The current approach of making the bitmap mutable is fine, though we've used builders in a lot of places in the project and I would've thought we'd do the same here. I didn't use a builder because I thought that we would want a mutable structure that can also function as a bitmap for reusing this later. As long as the implementation is almost entirely the same, it makes sense to me to use a bitmap API (`set`/`isSet`/`cardinality`) because it can be reused more. With a builder API it would be odd to check a builder if a particular bit is set, and setting a bit should return the builder for chaining instead of whether the bit was set (or was already set). > CoW is advantageous if we generally expect to modify a few containers and this optimization is well worth it for the general case, since that's when we avoid decoding the untouched ones. I think this matches our expectation that deletes will generally be sparse. The worst case is random deletes, where you're not targeting files that are clustered together. If you are targeting files that are clustered (say by date partition from writes) then deletes will cluster together and the copy-on-write strategy works better. But even for random deletes, a 1% delete rate means about 1 in 100 files will be affected, which touches about 1 container in 3. For a 50,000 entry file, there are about 200 containers. If a third of those containers is affected, that is 70 containers or about 2.2k allocated in overlay buffers. The alternative of maintaining a dense bitset is about 49k of memory. Plus you'd have to translate the existing entries by calling `set` a few thousand times. Maybe it isn't a big deal, but this cost adds up with the number of manifests as well. 100 manifests would cost about 5 MB, rather than 220k. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
