rohangarg commented on PR #13361: URL: https://github.com/apache/druid/pull/13361#issuecomment-1318492555
> This isn't really up to me, but we have offered to contribute if there is wider use beyond Druids needs, relevant slack thread https://the-asf.slack.com/archives/CP0930GKG/p1666290716186309. I guess I just used a ByteBuffer for this because I think such checks largely would negate the reasons of using unsafe, though I guess it would still allow larger memory objects than a java ByteBuffer allows to be done in a 'safe' manner if instead I was wrapping another Memory object, so maybe that is a better way if we were to merge this functionality in the upstream library since it would be a bit more flexible. I was mostly just being lazy as possible so I just used ByteBuffer since the checking was "free" to me. Is that what you meant? Thanks a lot for the link to the slack thread - it was relevant to the assertion/exception doubt that I had. Regarding the safe implementation, what I meant was to have a function like `Memory.safeWrap` which only our safe implementations use - the internal query system can keep on using the unsafe objects for better performance. Regarding the compatibility and safety, I agree with your points that the current modules are safe and we could put advisory in the docs for extension implementors to consider implementing this method and also ensure safety while ingesting intermediate states objects. My reasoning for suggesting flag was to not affect users who aren't using these functionalities but do have extensions - but agree that a default throwing implementation would be a better option than flag. -- 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]
