lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Edward Ribeiro (JIRA)" <>
Subject [jira] [Commented] (SOLR-12699) make LTRScoringModel immutable (to allow hashCode caching)
Date Mon, 03 Sep 2018 18:20:00 GMT


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:
List<String> mutableList = new ArrayList<>();

List<String> immutableList = Collections.unmodifiableList(mutableList);

System.out.println("immutableList = " + immutableList);


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

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message