You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/10/06 08:30:00 UTC
[GitHub] [lucene-solr] sigram opened a new pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
sigram opened a new pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951
This replaces nearly all Map / List usage in `MetricUtils` with `MapWriter` / `IteratorWriter`. The PR includes also changes to `Utils.MapWriterJSONWriter` to similarly avoid creating Map / List instances and instead serialize `MapWriter` / `IteratorWriter` directly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704215473
Can we get rid of the interface `PropertyFilter` and replace it with a `Predicate<String>`?
You can use the class `ConditionalKeyMapWriter` to filter
`MetricUtils.addMetrics()` : Why can't we just have an anonymous class that implements `MapWriter` and avoid creation of `NamedList`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram closed pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram closed pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#discussion_r501367533
##########
File path: solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
##########
@@ -1110,10 +1111,10 @@ private void addVal(NamedList<Object> nl, String key, Properties props, @Suppres
}
}
- private void addVal(Map<String, Object> map, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
+ private void addVal(MapWriter.EntryWriter ew, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
Object val = formatVal(key, props, clzz);
if (val != null) {
- map.put(key, val);
+ ew.putNoEx(key, val);
Review comment:
there is a `putIfNotNull()`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-705248728
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram closed pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram closed pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704676764
>Again, we're limited by back-compat in 8x. We could add another set of methods in MetricUtils that use Predicate<String> and deprecate PropertyFilter ... but that would double the number of methods.
That should be OK. We can double the no:of methods and deprecate the old one
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704677008
>Not in branch_8x - it's a part of the public API.
We can have both and mark the old as deprecated. The guages are creating too many Map objects
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#discussion_r500216190
##########
File path: solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
##########
@@ -308,19 +318,24 @@ public void testKeyMetrics() throws Exception {
handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
MetricsHandler.KEY_PARAM, key1, MetricsHandler.KEY_PARAM, key2, MetricsHandler.KEY_PARAM, key3), resp);
values = resp.getValues();
- val = values.findRecursive("metrics", key1);
+ map = new HashMap<>();
+ values.toMap(map);
+ val = Utils.getObjectByPath(map, false, "metrics/" + key1);
assertNotNull(val);
- val = values.findRecursive("metrics", key2);
+ val = Utils.getObjectByPath(map, true, "metrics/" + key2);
assertNotNull(val);
- val = values.findRecursive("metrics", key3);
+ val = Utils.getObjectByPath(map, true, "metrics/" + key3);
assertNotNull(val);
String key4 = "solr.core.collection1:QUERY./select.requestTimes:1minRate";
resp = new SolrQueryResponse();
handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json",
MetricsHandler.KEY_PARAM, key4), resp);
values = resp.getValues();
- val = values.findRecursive("metrics", key4);
+ map = new HashMap<>();
Review comment:
There is no need to convert this to `Map`.
`values._get()` can fetch nested values
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704446815
> Can we get rid of the interface PropertyFilter and replace it with a Predicate<String>?
Again, we're limited by back-compat in 8x. We could add another set of methods in `MetricUtils` that use `Predicate<String>` and deprecate `PropertyFilter` ... but that would double the number of methods.
> MetricUtils.addMetrics()
Not sure what you're referring to - this method doesn't create a NamedList. It's used only in `OverseerStatusCmd`, which creates a NamedList anyway for other stuff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-705484706
Merged - thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#discussion_r500206089
##########
File path: solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java
##########
@@ -385,10 +406,10 @@ static double nsToMs(boolean convert, double value) {
}
// some snapshots represent time in ns, other snapshots represent raw values (eg. chunk size)
- static void addSnapshot(Map<String, Object> response, Snapshot snapshot, PropertyFilter propertyFilter, boolean ms) {
+ static void addSnapshot(MapWriter.EntryWriter ew, Snapshot snapshot, PropertyFilter propertyFilter, boolean ms) {
BiConsumer<String, Object> filter = (k, v) -> {
if (propertyFilter.accept(k)) {
Review comment:
I see this pattern repeated in many places. I think a helper method can be added to avoid code duplication
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-705484706
Merged - thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#discussion_r501367533
##########
File path: solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
##########
@@ -1110,10 +1111,10 @@ private void addVal(NamedList<Object> nl, String key, Properties props, @Suppres
}
}
- private void addVal(Map<String, Object> map, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
+ private void addVal(MapWriter.EntryWriter ew, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
Object val = formatVal(key, props, clzz);
if (val != null) {
- map.put(key, val);
+ ew.putNoEx(key, val);
Review comment:
there is a putIfNotNull()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] sigram edited a comment on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
sigram edited a comment on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704446815
> Can we get rid of the interface PropertyFilter and replace it with a Predicate<String>?
Again, we're limited by back-compat in 8x. We could add another set of methods in `MetricUtils` that use `Predicate<String>` and deprecate `PropertyFilter` ... but that would double the number of methods.
> MetricUtils.addMetrics()
Not sure what you're referring to - this method doesn't create a NamedList. It's used only in `OverseerStatusCmd`, which creates a NamedList anyway for other stuff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1951: SOLR-14691: Reduce object creation by using MapWriter / IteratorWriter.
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1951:
URL: https://github.com/apache/lucene-solr/pull/1951#issuecomment-704237134
Is it possible to change `MetricsMap implements Gauge<Map<String,Object>>` to
` MetricsMap implements Gauge<MapWriter>`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org