[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873011#comment-16873011
]
Adrien Grand commented on LUCENE-8855:
--
+1
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872575#comment-16872575
]
Andrzej Bialecki commented on LUCENE-8855:
---
Updated patch:
* removed left-over code from BytesRefHash
* use UNKNOWN_DEFAULT_RAM_BYTES_USED = 256 for estimating unknown Objects.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872558#comment-16872558
]
Andrzej Bialecki commented on LUCENE-8855:
---
{quote}Could we make it fail if it encounters an unknown object
{quote}
It's not a strict calculation anyway so I don't think we should fail, esp.
since some queries (LTR) may legitimately contain arbitrary parameters - we
could simply assume a larger default value for unknown objects, equivalent eg.
to a class with a few primitive and a few moderately-sized String fields (256
bytes? 384 bytes?).
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872532#comment-16872532
]
Adrien Grand commented on LUCENE-8855:
--
BytesRefHash has some code commented out that looks like a left over? One minor
last concern I have is that sizeOf without a default size feels a bit trappy
since lots of objects are larger than their shallow size. Could we make it fail
if it encounters an unknown object instead of assuming shallow size?
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872513#comment-16872513
]
Andrzej Bialecki commented on LUCENE-8855:
---
[~jpountz] if the latest patch addresses all your comments I'd like to commit
it shortly.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16871502#comment-16871502
]
Andrzej Bialecki commented on LUCENE-8855:
---
Updated patch:
* removed Accountable from PointRangeQuery, BytesRef and IntsRef.
* use a fixed (relatively large) constant for estimating the size of unknown
query types.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch,
> LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868322#comment-16868322
]
Adrien Grand commented on LUCENE-8855:
--
Thanks Andrzej this looks like a better trade-off to me in general. Do we need
Accountable on PointRangeQuery, this one should always be small? I think we
should also avoid Accountable on BytesRef and IntsRef since these objects can
be used to represent a slice of an array. For instance I know in some of places
we have collections of BytesRef objects that all share the same byte[], so
counting the underlying byte[] more than once would be incorrect.
In the case of unknown queries I'm wondering whether we should return an
arbitrary constant instead of the shallow size of the object, in order to
overestimate memory usage instead of underestimating it? For the caching
use-case I suspect it's better to overestimate memory usage a bit?
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16867821#comment-16867821
]
Andrzej Bialecki commented on LUCENE-8855:
---
Updated patch:
* includes RamUsageEstimator improvements from previous patch
* uses QueryVisitor for traversing the Query tree
* adds Accountable to some of the classes that may be larger than default
estimate
* similarly, adds Accountable to Queries that may be much larger than the
default estimate (10x or more).
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16866893#comment-16866893
]
Andrzej Bialecki commented on LUCENE-8855:
---
Yes, this sounds like a good enough compromise. I'll work on a new patch.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16866445#comment-16866445
]
Adrien Grand commented on LUCENE-8855:
--
Would it be an option to use a visitor that assumes some constant memory usage
for the majority of queries (say 1kb to be conservative) and uses
Accountable#rambytesUsed for queries that implement Accountable? I agree it is
going to produce an estimation that will be less good, but maybe good enough in
order to not run out-of-memory because of queries as cache keys?
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16865699#comment-16865699
]
Andrzej Bialecki commented on LUCENE-8855:
---
[~jpountz] - it's true that in most cases nobody cares about Query memory
footprint, but the caching use case is an important one, and it needs more
support from the API in order to better estimate the actual cost of caching.
I don't think using Weight#isCacheable helps in this particular scenario
because you need to calculate Weight first, which brings its own significant
cost.
QueryVisitor API is an option, sure, but still the Query implementation itself
knows the best what its memory use is like, so it's still the Query impl that
needs to calculate this estimate - and then report it, either using a method
that's an extension of QueryVisitor or the already available Accountable.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16864783#comment-16864783
]
Atri Sharma commented on LUCENE-8855:
-
{quote}Or alternatively compute a rough estimate of the memory usage of queries
on top of the Query API by using the query visitor API and some heuristics.
{quote}
[~jpountz] [~ab] I have opened LUCENE-8864 to pursue this idea. Please share
your thoughts.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16864245#comment-16864245
]
Adrien Grand commented on LUCENE-8855:
--
I had the temptation to make Query implement Accountable a couple times over
the past years but I never liked it as it felt like something that is only
really useful in esoteric cases when memory usage of the cache keys - queries -
is not negligible compared to what is being cached. I'd rather not implement
Accountable.
My preference would be to either rely on the existing Weight#isCacheable logic,
which disables caching on large queries (see e.g.
https://github.com/apache/lucene-solr/blob/fbd05167f455e3ce2b2ead50336e2b9c2521cd6c/lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java#L337-L342)
so that queries that are used as cache keys are always small. Or alternatively
compute a rough estimate of the memory usage of queries on top of the Query API
by using the query visitor API and some heuristics.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16864097#comment-16864097
]
Atri Sharma commented on LUCENE-8855:
-
+1, important change!
I had a few comments/questions:
1) Can we make sizeOfObject method a bit more modular? I understand the idea
behind it, but it would be great if we could make it a bit more granular.
2) Should IntervalsQuery also account for the sizes of the sources?
On a side note, I am wondering if it makes sense to make RamUsageEstimator use
QueryVisitor API instead of iterating over all possible types of object types.
We would need to enhance QueryVisitor API to introduce a semantic of visiting
an Accountable object, but that should be a trivial change.
Of course, that is an orthogonal issue, and I will open a Jira for that.
Overall, +1 from me.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16863969#comment-16863969
]
Andrzej Bialecki commented on LUCENE-8855:
---
Hmm, apparently this issue is less contentious than I expected ;) If there are
no objections I'd like to commit this early next week.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16863502#comment-16863502
]
Andrzej Bialecki commented on LUCENE-8855:
---
Updated patch:
* add ramBytesUsed caching to some of the more complex (but finalized) queries
to avoid the runtime cost.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch, LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-8855) Add Accountable to Query implementations
[
https://issues.apache.org/jira/browse/LUCENE-8855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862460#comment-16862460
]
Andrzej Bialecki commented on LUCENE-8855:
---
This patch adds {{Accountable}} to all Query implementations and some of their
dependent classes. It also contains several additions to {{RamUsageEstimator}}
to make it easier to estimate the size of complex objects.
> Add Accountable to Query implementations
>
>
> Key: LUCENE-8855
> URL: https://issues.apache.org/jira/browse/LUCENE-8855
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Andrzej Bialecki
>Assignee: Andrzej Bialecki
>Priority: Major
> Attachments: LUCENE-8855.patch
>
>
> Query implementations should also support {{Accountable}} API in order to
> monitor the memory consumption e.g. in caches where either keys or values are
> {{Query}} instances.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
