[
https://issues.apache.org/jira/browse/SOLR-12699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16602383#comment-16602383
]
Edward Ribeiro commented on SOLR-12699:
---------------------------------------
Hi [~cpoerschke] and [~slivotov],
Just a nitpick: wrapping a collection in a Collections.unmodifiable* doesn't
make the collection strictly immutable. See, we are receiving `features` as a
parameter. Therefore, some code that still has a reference to that object can
change it underneath. Example:
{code:java}
List<String> mutableList = new ArrayList<>();
mutableList.add("Hello");
mutableList.add("World");
List<String> immutableList = Collections.unmodifiableList(mutableList);
System.out.println("immutableList = " + immutableList);
mutableList.add("!");
System.out.println("list2 = " + immutableList);{code}
As collections is a wrapper I could change it by modifying the `mutableList`.
AFAIK, to make it a real immutable collection it would be required to create a
defensive copy of the collection like below:
{code:java}
List<String> immutableList = Collections.unmodifiableList(new
ArrayList<>(mutableList));{code}
Now the elements of mutableList are copied to a new ArrayList that we don't
have reference. Therefore we cannot change its elements even tough as you
pointed out, we could change fields of the objects in the collection. *The
problem with this approach is that if `features` is a large collection this
could impact the performance, so be cautions when using it.*
Finally, this line
{code:java}
this.features = features != null ? Collections.unmodifiableList(features) :
features;{code}
`this.features` receives `null` if `features` is `null`. Is that the intended
behaviour? Wouldn't be better to make it be `Lists.emptyList()` if `features`
is null? Excuse me if I am missing something, but it's usually an anti-pattern
to return null, but I am very well aware that the codebases in the wild usually
don't follow this advice.
> make LTRScoringModel immutable (to allow hashCode caching)
> ----------------------------------------------------------
>
> Key: SOLR-12699
> URL: https://issues.apache.org/jira/browse/SOLR-12699
> Project: Solr
> Issue Type: Sub-task
> Security Level: Public(Default Security Level. Issues are Public)
> Components: contrib - LTR
> Reporter: Stanislav Livotov
> Priority: Major
> Attachments: SOLR-12699.patch, SOLR-12699.patch, SOLR-12699.patch
>
>
> [~slivotov] wrote in SOLR-12688:
> bq. ... LTRScoringModel was a mutable object. It was leading to the
> calculation of hashcode on each query, which in turn can consume a lot of
> time ... So I decided to make LTRScoringModel immutable and cache hashCode
> calculation. ...
> (Please see SOLR-12688 description for overall context and analysis results.)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
