From f51340993aea7cca3053844284c115bddaa90215 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Fri, 21 Oct 2016 18:58:33 +0530 Subject: [PATCH] SOLR-9546: Eliminate unnecessary boxing/unboxing going on in SolrParams --- solr/CHANGES.txt | 2 + .../org/apache/solr/cloud/ZkController.java | 2 +- .../solr/handler/DumpRequestHandler.java | 6 +- .../solr/handler/ReplicationHandler.java | 2 +- .../solr/response/BinaryResponseWriter.java | 3 +- .../solr/response/JSONResponseWriter.java | 3 +- .../apache/solr/search/HashQParserPlugin.java | 4 +- .../TextLogisticRegressionQParserPlugin.java | 2 +- .../solr/search/mlt/CloudMLTQParser.java | 29 ++-- .../solr/search/mlt/SimpleMLTQParser.java | 31 ++--- .../apache/solr/common/params/SolrParams.java | 124 ++++++++++++++---- 11 files changed, 132 insertions(+), 76 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 1b1c9cbf226..43384fe68db 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -184,6 +184,8 @@ Optimizations * SOLR-9566: Don't put replicas into recovery when first creating a Collection (Alan Woodward) +* SOLR-9546: Eliminate unnecessary boxing/unboxing going on in SolrParams (Pushkar Raste, noble) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 9b0a90e5875..c0a8d555000 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -1887,7 +1887,7 @@ public class ZkController { elect.setup(context); electionContexts.put(contextKey, context); - elect.retryElection(context, params.getBool(REJOIN_AT_HEAD_PROP)); + elect.retryElection(context, params.getBool(REJOIN_AT_HEAD_PROP, false)); } } catch (Exception e) { throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to rejoin election", e); diff --git a/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java index 9b0f9599c8d..ecafb526d29 100644 --- a/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java @@ -56,13 +56,15 @@ public class DumpRequestHandler extends RequestHandlerBase } } - if(Boolean.TRUE.equals( req.getParams().getBool("getdefaults"))){ + if(req.getParams().getBool("getdefaults", false)){ NamedList def = (NamedList) initArgs.get(PluginInfo.DEFAULTS); rsp.add("getdefaults", def); } - if(Boolean.TRUE.equals( req.getParams().getBool("initArgs"))) rsp.add("initArgs", initArgs); + if(req.getParams().getBool("initArgs", false)) { + rsp.add("initArgs", initArgs); + } // Write the streams... if( req.getContentStreams() != null ) { diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index 759b7c6af14..d56d527b3f2 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java @@ -1431,7 +1431,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw sLen = params.get(LEN); compress = params.get(COMPRESSION); useChecksum = params.getBool(CHECKSUM, false); - indexGen = params.getLong(GENERATION, null); + indexGen = params.getLong(GENERATION); if (useChecksum) { checksum = new Adler32(); } diff --git a/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java b/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java index 9634e63b176..11c60740cb0 100644 --- a/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java @@ -47,8 +47,7 @@ public class BinaryResponseWriter implements BinaryQueryResponseWriter { @Override public void write(OutputStream out, SolrQueryRequest req, SolrQueryResponse response) throws IOException { Resolver resolver = new Resolver(req, response.getReturnFields()); - Boolean omitHeader = req.getParams().getBool(CommonParams.OMIT_HEADER); - if (omitHeader != null && omitHeader) response.removeResponseHeader(); + if (req.getParams().getBool(CommonParams.OMIT_HEADER, false)) response.removeResponseHeader(); new JavaBinCodec(resolver).setWritableDocFields(resolver).marshal(response.getValues(), out); } diff --git a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java index 522030fbe0f..cd6648bf111 100644 --- a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java @@ -92,8 +92,7 @@ class JSONWriter extends TextResponseWriter { if(wrapperFunction!=null) { writer.write(wrapperFunction + "("); } - Boolean omitHeader = req.getParams().getBool(CommonParams.OMIT_HEADER); - if(omitHeader != null && omitHeader) rsp.removeResponseHeader(); + if(req.getParams().getBool(CommonParams.OMIT_HEADER, false)) rsp.removeResponseHeader(); writeNamedList(null, rsp.getValues()); if(wrapperFunction!=null) { writer.write(')'); diff --git a/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java index 2112c714446..db0223a7712 100644 --- a/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java @@ -66,8 +66,8 @@ public class HashQParserPlugin extends QParserPlugin { } public Query parse() { - int workers = localParams.getInt("workers"); - int worker = localParams.getInt("worker"); + int workers = localParams.getInt("workers", 0); + int worker = localParams.getInt("worker", 0); String keys = params.get("partitionKeys"); keys = keys.replace(" ", ""); return new HashQuery(keys, workers, worker); diff --git a/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java index 0ca9f723f32..2b316ca90a5 100644 --- a/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/TextLogisticRegressionQParserPlugin.java @@ -74,7 +74,7 @@ public class TextLogisticRegressionQParserPlugin extends QParserPlugin { String[] terms = params.get("terms").split(","); String ws = params.get("weights"); String dfsStr = params.get("idfs"); - int iteration = params.getInt("iteration"); + int iteration = params.getInt("iteration", 0); String outcome = params.get("outcome"); int positiveLabel = params.getInt("positiveLabel", 1); double threshold = params.getDouble("threshold", 0.5); diff --git a/solr/core/src/java/org/apache/solr/search/mlt/CloudMLTQParser.java b/solr/core/src/java/org/apache/solr/search/mlt/CloudMLTQParser.java index 74916530eca..2a64527bea6 100644 --- a/solr/core/src/java/org/apache/solr/search/mlt/CloudMLTQParser.java +++ b/solr/core/src/java/org/apache/solr/search/mlt/CloudMLTQParser.java @@ -69,29 +69,16 @@ public class CloudMLTQParser extends QParser { Map boostFields = new HashMap<>(); MoreLikeThis mlt = new MoreLikeThis(req.getSearcher().getIndexReader()); - if(localParams.getInt("mintf") != null) - mlt.setMinTermFreq(localParams.getInt("mintf")); - - mlt.setMinDocFreq(localParams.getInt("mindf", 0)); - - if(localParams.get("minwl") != null) - mlt.setMinWordLen(localParams.getInt("minwl")); - - if(localParams.get("maxwl") != null) - mlt.setMaxWordLen(localParams.getInt("maxwl")); - - if(localParams.get("maxqt") != null) - mlt.setMaxQueryTerms(localParams.getInt("maxqt")); - - if(localParams.get("maxntp") != null) - mlt.setMaxNumTokensParsed(localParams.getInt("maxntp")); - - if(localParams.get("maxdf") != null) { - mlt.setMaxDocFreq(localParams.getInt("maxdf")); - } + mlt.setMinTermFreq(localParams.getInt("mintf", MoreLikeThis.DEFAULT_MIN_TERM_FREQ)); + mlt.setMinDocFreq(localParams.getInt("mindf", MoreLikeThis.DEFAULT_MIN_DOC_FREQ)); + mlt.setMinWordLen(localParams.getInt("minwl", MoreLikeThis.DEFAULT_MIN_WORD_LENGTH)); + mlt.setMaxWordLen(localParams.getInt("maxwl", MoreLikeThis.DEFAULT_MIN_WORD_LENGTH)); + mlt.setMaxQueryTerms(localParams.getInt("maxqt",MoreLikeThis.DEFAULT_MAX_QUERY_TERMS)); + mlt.setMaxNumTokensParsed(localParams.getInt("maxntp",MoreLikeThis.DEFAULT_MAX_NUM_TOKENS_PARSED)); + mlt.setMaxDocFreq(localParams.getInt("maxdf", MoreLikeThis.DEFAULT_MAX_DOC_FREQ)); if(localParams.get("boost") != null) { - mlt.setBoost(localParams.getBool("boost")); + mlt.setBoost(localParams.getBool("boost", false)); boostFields = SolrPluginUtils.parseFieldBoosts(qf); } diff --git a/solr/core/src/java/org/apache/solr/search/mlt/SimpleMLTQParser.java b/solr/core/src/java/org/apache/solr/search/mlt/SimpleMLTQParser.java index bdcaafbf670..d4403ad9f1e 100644 --- a/solr/core/src/java/org/apache/solr/search/mlt/SimpleMLTQParser.java +++ b/solr/core/src/java/org/apache/solr/search/mlt/SimpleMLTQParser.java @@ -69,30 +69,17 @@ public class SimpleMLTQParser extends QParser { ScoreDoc[] scoreDocs = td.scoreDocs; MoreLikeThis mlt = new MoreLikeThis(req.getSearcher().getIndexReader()); - if(localParams.getInt("mintf") != null) - mlt.setMinTermFreq(localParams.getInt("mintf")); - - if(localParams.getInt("mindf") != null) - mlt.setMinDocFreq(localParams.getInt("mindf")); - - if(localParams.get("minwl") != null) - mlt.setMinWordLen(localParams.getInt("minwl")); - - if(localParams.get("maxwl") != null) - mlt.setMaxWordLen(localParams.getInt("maxwl")); - - if(localParams.get("maxqt") != null) - mlt.setMaxQueryTerms(localParams.getInt("maxqt")); - - if(localParams.get("maxntp") != null) - mlt.setMaxNumTokensParsed(localParams.getInt("maxntp")); - - if(localParams.get("maxdf") != null) { - mlt.setMaxDocFreq(localParams.getInt("maxdf")); - } + mlt.setMinTermFreq(localParams.getInt("mintf", MoreLikeThis.DEFAULT_MIN_TERM_FREQ)); + mlt.setMinDocFreq(localParams.getInt("mindf", MoreLikeThis.DEFAULT_MIN_DOC_FREQ)); + mlt.setMinWordLen(localParams.getInt("minwl", MoreLikeThis.DEFAULT_MIN_WORD_LENGTH)); + mlt.setMaxWordLen(localParams.getInt("maxwl", MoreLikeThis.DEFAULT_MAX_WORD_LENGTH)); + mlt.setMaxQueryTerms(localParams.getInt("maxqt", MoreLikeThis.DEFAULT_MAX_QUERY_TERMS)); + mlt.setMaxNumTokensParsed(localParams.getInt("maxntp", MoreLikeThis.DEFAULT_MAX_NUM_TOKENS_PARSED)); + mlt.setMaxDocFreq(localParams.getInt("maxdf", MoreLikeThis.DEFAULT_MAX_DOC_FREQ)); + // what happens if value is explicitly set to false? if(localParams.get("boost") != null) { - mlt.setBoost(localParams.getBool("boost")); + mlt.setBoost(localParams.getBool("boost", false)); boostFields = SolrPluginUtils.parseFieldBoosts(qf); } diff --git a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java index 0b74c1486e3..e884a5b0ebd 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java @@ -92,11 +92,24 @@ public abstract class SolrParams implements Serializable, MapSerializable { return val!=null ? val : getParams(param); } - /** Returns the Boolean value of the param, or null if not set */ + /** + * Returns the Boolean value of the param, or null if not set. + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value false. + * @see #getBool(String, boolean) + * @see #getPrimitiveBool(String) + * + **/ + public Boolean getBool(String param) { String val = get(param); return val==null ? null : StrUtils.parseBool(val); } + + /** Returns the boolean value of the param, or false if not set */ + public boolean getPrimitiveBool(String param) { + return getBool(param, false); + } /** Returns the boolean value of the param, or def if not set */ public boolean getBool(String param, boolean def) { @@ -104,21 +117,46 @@ public abstract class SolrParams implements Serializable, MapSerializable { return val==null ? def : StrUtils.parseBool(val); } - /** Returns the Boolean value of the field param, - or the value for param, or null if neither is set. */ + /** + * Returns the Boolean value of the field param, + * or the value for param, or null if neither is set. + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value false. + * @see #getFieldBool(String, String, boolean) + * @see #getPrimitiveFieldBool(String, String) + **/ public Boolean getFieldBool(String field, String param) { String val = getFieldParam(field, param); return val==null ? null : StrUtils.parseBool(val); } + + /** + * Returns the boolean value of the field param, or + * the value for param or + * the default value of boolean - false + */ + public boolean getPrimitiveFieldBool(String field, String param) { + return getFieldBool(field, param, false); + } - /** Returns the boolean value of the field param, - or the value for param, or def if neither is set. */ + /** + * Returns the boolean value of the field param, + * or the value for param, or def if neither is set. + * + * */ public boolean getFieldBool(String field, String param, boolean def) { String val = getFieldParam(field, param); return val==null ? def : StrUtils.parseBool(val); } - /** Returns the Integer value of the param, or null if not set */ + /** + * Returns the Integer value of the param, or null if not set + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value for int - + * zero (0). + * @see #getInt(String, int) + * @see #getPrimitiveInt(String) + * */ public Integer getInt(String param) { String val = get(param); try { @@ -128,30 +166,33 @@ public abstract class SolrParams implements Serializable, MapSerializable { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, ex.getMessage(), ex ); } } - - /** Returns the Long value of the param, or null if not set */ - public Long getLong(String param, Long def) { - String val = get(param); - try { - return val== null ? def : Long.parseLong(val); - } - catch( Exception ex ) { - throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, ex.getMessage(), ex ); - } + + /** + * Returns int value of the the param or + * default value for int - zero (0) if not set. + */ + public int getPrimitiveInt(String param) { + return getInt(param, 0); } /** Returns the int value of the param, or def if not set */ public int getInt(String param, int def) { String val = get(param); try { - return val==null ? def : Integer.parseInt(val); + return val == null ? def : Integer.parseInt(val); } catch( Exception ex ) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, ex.getMessage(), ex ); } } - /** Returns the Long value of the param, or null if not set */ + /** + * Returns the Long value of the param, or null if not set + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0). + * @see #getLong(String, long) + * + **/ public Long getLong(String param) { String val = get(param); try { @@ -173,8 +214,13 @@ public abstract class SolrParams implements Serializable, MapSerializable { /** + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0). + * * @return The int value of the field param, or the value for param * or null if neither is set. + * + * @see #getFieldInt(String, String, int) **/ public Integer getFieldInt(String field, String param) { String val = getFieldParam(field, param); @@ -199,7 +245,12 @@ public abstract class SolrParams implements Serializable, MapSerializable { } - /** Returns the Float value of the param, or null if not set */ + /** + * Returns the Float value of the param, or null if not set + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0.0f). + * @see #getFloat(String, float) + **/ public Float getFloat(String param) { String val = get(param); try { @@ -221,7 +272,13 @@ public abstract class SolrParams implements Serializable, MapSerializable { } } - /** Returns the Float value of the param, or null if not set */ + /** + * Returns the Float value of the param, or null if not set + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0.0d). + * @see #getDouble(String, double) + * + **/ public Double getDouble(String param) { String val = get(param); try { @@ -244,7 +301,15 @@ public abstract class SolrParams implements Serializable, MapSerializable { } - /** Returns the float value of the field param. */ + /** + * Returns the float value of the field param. + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0.0f). + * + * @see #getFieldFloat(String, String, float) + * @see #getPrimitiveFieldFloat(String, String) + * + **/ public Float getFieldFloat(String field, String param) { String val = getFieldParam(field, param); try { @@ -254,6 +319,15 @@ public abstract class SolrParams implements Serializable, MapSerializable { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, ex.getMessage(), ex ); } } + + /** + * Returns the float value of the field param or + * the value for param or + * the default value for float - zero (0.0f) + */ + public float getPrimitiveFieldFloat(String field, String param) { + return getFieldFloat(field, param, 0.0f); + } /** Returns the float value of the field param, or the value for param, or def if neither is set. */ @@ -267,7 +341,13 @@ public abstract class SolrParams implements Serializable, MapSerializable { } } - /** Returns the float value of the field param. */ + /** + * Returns the float value of the field param. + * Use this method only when you want to be explicit + * about absence of a value (null) vs the default value zero (0.0d). + * @see #getDouble(String, double) + * + **/ public Double getFieldDouble(String field, String param) { String val = getFieldParam(field, param); try {