pitrou commented on code in PR #40041:
URL: https://github.com/apache/arrow/pull/40041#discussion_r1486495424


##########
cpp/src/gandiva/cache.cc:
##########
@@ -23,11 +23,7 @@
 
 namespace gandiva {
 
-#ifdef GANDIVA_ENABLE_OBJECT_CODE_CACHE
 static const size_t DEFAULT_CACHE_SIZE = 500000;

Review Comment:
   There's a problem here: what is the unit? The large number seems to imply 
this is expressed in bytes.
   However, the size will actually be interpreted as the number of stored 
entries (it's passed to `LruCache` which in turn compares it to 
`std::unordered_map::size`).
   
   It would probably be beneficial to cap the number of bytes and not the 
number of entries, but this is not what this PR does. 500000 cache entries may 
easily add up to hundreds of megabytes of RAM.
   



-- 
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]

Reply via email to