You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Paulex Yang <pa...@gmail.com> on 2006/03/15 10:10:39 UTC

[classlib]bug-to-bug compatibility: HashMap

Pls. try the test case on HashMap below.

On RI, it print out:
null
null

On Harmony, it print out
null
value1

it is definitely bad practice to reuse a object as hash key by modify 
its hashcode, but I DID see some similar cases before, after all, you 
cannot force but only can *suggest* developers what to do.

So, what should we do?  Try to replicate RI's behavior?


public class HashMapTest {
 public static void main(String[] args) {
  ReusableKey k = new ReusableKey();
  HashMap map = new HashMap();
  k.setKey(1);
  map.put(k, "value1");
 
  k.setKey(18);
  //both RI and Harmony get null here
  System.out.println(map.get(k));
 
  k.setKey(17);
  //RI get null
  //Harmony get "value1"
  //the number of 17 is *magic* because the default length of HashMap is 16.
  System.out.println(map.get(k));
 }
}

class ReusableKey{
 private int key = 0;
 public void setKey(int key){
  this.key = key;
 }
 public int hashCode(){
  return key;
 }
 public boolean equals(Object o){
  if(o == this){
   return true;
  }
  if(!(o instanceof ReusableKey)){
   return false;
  }
  return key == ((ReusableKey)o).key;
 }
}


-- 
Paulex Yang
China Software Development Lab
IBM



Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Tim Ellison <t....@gmail.com>.
Mikhail Loenko wrote:
> Geir,
> 
> 2006/3/19, Geir Magnusson Jr <ge...@pobox.com>:
>> Did we ever come to consensus on how to track these?
>>
>> We are making a very deliberate and reasonable decision here to not be
>> spec compliant, right?
> 
> not exactly. The spec says that "behavior of a map is not specified"
> so anything we do is spec compliant.
> 
> what we did is we chose the best behavior from many compliant ones

well, the proposal on the table is to 'copy the reference implementation
behavior' (if that is how you choose to define 'best').

Regards,
Tim

> Thanks
> Mikhail
> 
>> I really would like us to note this.  I'll be happy to do it of course,
>> but that won't scale - we should come to agreement on how we want to
>> handle this, and make it the responsibliity of every committer to do the
>> Right Thing when this happens in the future.
>>
>> geir
>>
>>
>> Paulex Yang wrote:
>>> Hi,
>>>
>>> I've raised JIRA-206 and attached patch to comply with RI.
>>>
>>> Richard Liang wrote:
>>>> Hello Paulex,
>>>>
>>>> According to Java 5 Spec, "Note: great care must be exercised if
>>>> mutable objects are used as map keys. The behavior of a map is not
>>>> specified if the value of an object is changed in a manner that
>>>> affects equals comparisons while the object is a key in the map...."
>>>>
>>>> As the behavior of a map is not specified, we can either keep
>>>> compatible with RI, or leave it as is. But keeping compatible with RI
>>>> may be a better solution. :-)
>>>>
>>>> Paulex Yang wrote:
>>>>> Hi, Mikhail,
>>>>>
>>>>> Mikhail Loenko wrote:
>>>>>> Hi Paulex,
>>>>>>
>>>>>> IMHO from compatibility point of view any behavior is legal.
>>>>>> HashMap is mostly designed for keys that do not change over time, at
>>>>>> least
>>>>>> spec silent about changing the keys. So, I think implementation
>>>>>> specsific is
>>>>>> allowed here.
>>>>>>
>>>>>> But I think that aiming stability we'd better compare current hash
>>>>>> with the original one - in this case get() result will depend on the
>>>>>> hash
>>>>>> rather that on the inernal state of HashMap (such as HashMap length)
>>>>>>
>>>>> I'm afraid the problem here is not the HashMap length,  whatever the
>>>>> length is, this kind of problem can happen, 16 just make it more
>>>>> easier, because it is not a prime number.
>>>>>
>>>>> So if my understanding is right, you suggest to cache the key's hash
>>>>> value as *original* hash into HashMap.Entry, when the key/value pair
>>>>> is put into HashMap; and when someone want to get back the value from
>>>>> HashMap by the same key, the cached *original* hash will be compared
>>>>> with the *current* hash, and if they are not equal, HashMap will
>>>>> consider them as non-match, even the keys are *same* object. In fact
>>>>> that's also my point of view to fix this non-compatibility. This fix
>>>>> is not difficult, but the only concern is, this behaviour may be
>>>>> controversial.
>>>>>
>>>>> thoughts?
>>>>>> Thanks,
>>>>>> Mikhail
>>>>>>
>>>>>> 2006/3/15, Paulex Yang <pa...@gmail.com>:
>>>>>>
>>>>>>> Pls. try the test case on HashMap below.
>>>>>>>
>>>>>>> On RI, it print out:
>>>>>>> null
>>>>>>> null
>>>>>>>
>>>>>>> On Harmony, it print out
>>>>>>> null
>>>>>>> value1
>>>>>>>
>>>>>>> it is definitely bad practice to reuse a object as hash key by modify
>>>>>>> its hashcode, but I DID see some similar cases before, after all, you
>>>>>>> cannot force but only can *suggest* developers what to do.
>>>>>>>
>>>>>>> So, what should we do?  Try to replicate RI's behavior?
>>>>>>>
>>>>>>>
>>>>>>> public class HashMapTest {
>>>>>>>  public static void main(String[] args) {
>>>>>>>  ReusableKey k = new ReusableKey();
>>>>>>>  HashMap map = new HashMap();
>>>>>>>  k.setKey(1);
>>>>>>>  map.put(k, "value1");
>>>>>>>
>>>>>>>  k.setKey(18);
>>>>>>>  //both RI and Harmony get null here
>>>>>>>  System.out.println(map.get(k));
>>>>>>>
>>>>>>>  k.setKey(17);
>>>>>>>  //RI get null
>>>>>>>  //Harmony get "value1"
>>>>>>>  //the number of 17 is *magic* because the default length of
>>>>>>> HashMap is 16.
>>>>>>>  System.out.println(map.get(k));
>>>>>>>  }
>>>>>>> }
>>>>>>>
>>>>>>> class ReusableKey{
>>>>>>>  private int key = 0;
>>>>>>>  public void setKey(int key){
>>>>>>>  this.key = key;
>>>>>>>  }
>>>>>>>  public int hashCode(){
>>>>>>>  return key;
>>>>>>>  }
>>>>>>>  public boolean equals(Object o){
>>>>>>>  if(o == this){
>>>>>>>   return true;
>>>>>>>  }
>>>>>>>  if(!(o instanceof ReusableKey)){
>>>>>>>   return false;
>>>>>>>  }
>>>>>>>  return key == ((ReusableKey)o).key;
>>>>>>>  }
>>>>>>> }
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Paulex Yang
>>>>>>> China Software Development Lab
>>>>>>> IBM
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
> 

-- 

Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Mikhail Loenko <ml...@gmail.com>.
Geir,

2006/3/19, Geir Magnusson Jr <ge...@pobox.com>:
> Did we ever come to consensus on how to track these?
>
> We are making a very deliberate and reasonable decision here to not be
> spec compliant, right?

not exactly. The spec says that "behavior of a map is not specified"
so anything we do is spec compliant.

what we did is we chose the best behavior from many compliant ones

Thanks
Mikhail

>
> I really would like us to note this.  I'll be happy to do it of course,
> but that won't scale - we should come to agreement on how we want to
> handle this, and make it the responsibliity of every committer to do the
> Right Thing when this happens in the future.
>
> geir
>
>
> Paulex Yang wrote:
> > Hi,
> >
> > I've raised JIRA-206 and attached patch to comply with RI.
> >
> > Richard Liang wrote:
> >> Hello Paulex,
> >>
> >> According to Java 5 Spec, "Note: great care must be exercised if
> >> mutable objects are used as map keys. The behavior of a map is not
> >> specified if the value of an object is changed in a manner that
> >> affects equals comparisons while the object is a key in the map...."
> >>
> >> As the behavior of a map is not specified, we can either keep
> >> compatible with RI, or leave it as is. But keeping compatible with RI
> >> may be a better solution. :-)
> >>
> >> Paulex Yang wrote:
> >>> Hi, Mikhail,
> >>>
> >>> Mikhail Loenko wrote:
> >>>> Hi Paulex,
> >>>>
> >>>> IMHO from compatibility point of view any behavior is legal.
> >>>> HashMap is mostly designed for keys that do not change over time, at
> >>>> least
> >>>> spec silent about changing the keys. So, I think implementation
> >>>> specsific is
> >>>> allowed here.
> >>>>
> >>>> But I think that aiming stability we'd better compare current hash
> >>>> with the original one - in this case get() result will depend on the
> >>>> hash
> >>>> rather that on the inernal state of HashMap (such as HashMap length)
> >>>>
> >>> I'm afraid the problem here is not the HashMap length,  whatever the
> >>> length is, this kind of problem can happen, 16 just make it more
> >>> easier, because it is not a prime number.
> >>>
> >>> So if my understanding is right, you suggest to cache the key's hash
> >>> value as *original* hash into HashMap.Entry, when the key/value pair
> >>> is put into HashMap; and when someone want to get back the value from
> >>> HashMap by the same key, the cached *original* hash will be compared
> >>> with the *current* hash, and if they are not equal, HashMap will
> >>> consider them as non-match, even the keys are *same* object. In fact
> >>> that's also my point of view to fix this non-compatibility. This fix
> >>> is not difficult, but the only concern is, this behaviour may be
> >>> controversial.
> >>>
> >>> thoughts?
> >>>> Thanks,
> >>>> Mikhail
> >>>>
> >>>> 2006/3/15, Paulex Yang <pa...@gmail.com>:
> >>>>
> >>>>> Pls. try the test case on HashMap below.
> >>>>>
> >>>>> On RI, it print out:
> >>>>> null
> >>>>> null
> >>>>>
> >>>>> On Harmony, it print out
> >>>>> null
> >>>>> value1
> >>>>>
> >>>>> it is definitely bad practice to reuse a object as hash key by modify
> >>>>> its hashcode, but I DID see some similar cases before, after all, you
> >>>>> cannot force but only can *suggest* developers what to do.
> >>>>>
> >>>>> So, what should we do?  Try to replicate RI's behavior?
> >>>>>
> >>>>>
> >>>>> public class HashMapTest {
> >>>>>  public static void main(String[] args) {
> >>>>>  ReusableKey k = new ReusableKey();
> >>>>>  HashMap map = new HashMap();
> >>>>>  k.setKey(1);
> >>>>>  map.put(k, "value1");
> >>>>>
> >>>>>  k.setKey(18);
> >>>>>  //both RI and Harmony get null here
> >>>>>  System.out.println(map.get(k));
> >>>>>
> >>>>>  k.setKey(17);
> >>>>>  //RI get null
> >>>>>  //Harmony get "value1"
> >>>>>  //the number of 17 is *magic* because the default length of
> >>>>> HashMap is 16.
> >>>>>  System.out.println(map.get(k));
> >>>>>  }
> >>>>> }
> >>>>>
> >>>>> class ReusableKey{
> >>>>>  private int key = 0;
> >>>>>  public void setKey(int key){
> >>>>>  this.key = key;
> >>>>>  }
> >>>>>  public int hashCode(){
> >>>>>  return key;
> >>>>>  }
> >>>>>  public boolean equals(Object o){
> >>>>>  if(o == this){
> >>>>>   return true;
> >>>>>  }
> >>>>>  if(!(o instanceof ReusableKey)){
> >>>>>   return false;
> >>>>>  }
> >>>>>  return key == ((ReusableKey)o).key;
> >>>>>  }
> >>>>> }
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Paulex Yang
> >>>>> China Software Development Lab
> >>>>> IBM
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>>
> >>
> >>
> >
> >
>

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Geir Magnusson Jr <ge...@pobox.com>.
Did we ever come to consensus on how to track these?

We are making a very deliberate and reasonable decision here to not be 
spec compliant, right?

I really would like us to note this.  I'll be happy to do it of course, 
but that won't scale - we should come to agreement on how we want to 
handle this, and make it the responsibliity of every committer to do the 
Right Thing when this happens in the future.

geir


Paulex Yang wrote:
> Hi,
> 
> I've raised JIRA-206 and attached patch to comply with RI.
> 
> Richard Liang wrote:
>> Hello Paulex,
>>
>> According to Java 5 Spec, "Note: great care must be exercised if 
>> mutable objects are used as map keys. The behavior of a map is not 
>> specified if the value of an object is changed in a manner that 
>> affects equals comparisons while the object is a key in the map...."
>>
>> As the behavior of a map is not specified, we can either keep 
>> compatible with RI, or leave it as is. But keeping compatible with RI 
>> may be a better solution. :-)
>>
>> Paulex Yang wrote:
>>> Hi, Mikhail,
>>>
>>> Mikhail Loenko wrote:
>>>> Hi Paulex,
>>>>
>>>> IMHO from compatibility point of view any behavior is legal.
>>>> HashMap is mostly designed for keys that do not change over time, at 
>>>> least
>>>> spec silent about changing the keys. So, I think implementation 
>>>> specsific is
>>>> allowed here.
>>>>
>>>> But I think that aiming stability we'd better compare current hash
>>>> with the original one - in this case get() result will depend on the 
>>>> hash
>>>> rather that on the inernal state of HashMap (such as HashMap length)
>>>>   
>>> I'm afraid the problem here is not the HashMap length,  whatever the 
>>> length is, this kind of problem can happen, 16 just make it more 
>>> easier, because it is not a prime number.
>>>
>>> So if my understanding is right, you suggest to cache the key's hash 
>>> value as *original* hash into HashMap.Entry, when the key/value pair 
>>> is put into HashMap; and when someone want to get back the value from 
>>> HashMap by the same key, the cached *original* hash will be compared 
>>> with the *current* hash, and if they are not equal, HashMap will 
>>> consider them as non-match, even the keys are *same* object. In fact 
>>> that's also my point of view to fix this non-compatibility. This fix 
>>> is not difficult, but the only concern is, this behaviour may be 
>>> controversial.
>>>
>>> thoughts?
>>>> Thanks,
>>>> Mikhail
>>>>
>>>> 2006/3/15, Paulex Yang <pa...@gmail.com>:
>>>>  
>>>>> Pls. try the test case on HashMap below.
>>>>>
>>>>> On RI, it print out:
>>>>> null
>>>>> null
>>>>>
>>>>> On Harmony, it print out
>>>>> null
>>>>> value1
>>>>>
>>>>> it is definitely bad practice to reuse a object as hash key by modify
>>>>> its hashcode, but I DID see some similar cases before, after all, you
>>>>> cannot force but only can *suggest* developers what to do.
>>>>>
>>>>> So, what should we do?  Try to replicate RI's behavior?
>>>>>
>>>>>
>>>>> public class HashMapTest {
>>>>>  public static void main(String[] args) {
>>>>>  ReusableKey k = new ReusableKey();
>>>>>  HashMap map = new HashMap();
>>>>>  k.setKey(1);
>>>>>  map.put(k, "value1");
>>>>>
>>>>>  k.setKey(18);
>>>>>  //both RI and Harmony get null here
>>>>>  System.out.println(map.get(k));
>>>>>
>>>>>  k.setKey(17);
>>>>>  //RI get null
>>>>>  //Harmony get "value1"
>>>>>  //the number of 17 is *magic* because the default length of 
>>>>> HashMap is 16.
>>>>>  System.out.println(map.get(k));
>>>>>  }
>>>>> }
>>>>>
>>>>> class ReusableKey{
>>>>>  private int key = 0;
>>>>>  public void setKey(int key){
>>>>>  this.key = key;
>>>>>  }
>>>>>  public int hashCode(){
>>>>>  return key;
>>>>>  }
>>>>>  public boolean equals(Object o){
>>>>>  if(o == this){
>>>>>   return true;
>>>>>  }
>>>>>  if(!(o instanceof ReusableKey)){
>>>>>   return false;
>>>>>  }
>>>>>  return key == ((ReusableKey)o).key;
>>>>>  }
>>>>> }
>>>>>
>>>>>
>>>>> -- 
>>>>> Paulex Yang
>>>>> China Software Development Lab
>>>>> IBM
>>>>>
>>>>>
>>>>>
>>>>>     
>>>>
>>>>   
>>>
>>>
>>
>>
> 
> 

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Paulex Yang <pa...@gmail.com>.
Hi,

I've raised JIRA-206 and attached patch to comply with RI.

Richard Liang wrote:
> Hello Paulex,
>
> According to Java 5 Spec, "Note: great care must be exercised if 
> mutable objects are used as map keys. The behavior of a map is not 
> specified if the value of an object is changed in a manner that 
> affects equals comparisons while the object is a key in the map...."
>
> As the behavior of a map is not specified, we can either keep 
> compatible with RI, or leave it as is. But keeping compatible with RI 
> may be a better solution. :-)
>
> Paulex Yang wrote:
>> Hi, Mikhail,
>>
>> Mikhail Loenko wrote:
>>> Hi Paulex,
>>>
>>> IMHO from compatibility point of view any behavior is legal.
>>> HashMap is mostly designed for keys that do not change over time, at 
>>> least
>>> spec silent about changing the keys. So, I think implementation 
>>> specsific is
>>> allowed here.
>>>
>>> But I think that aiming stability we'd better compare current hash
>>> with the original one - in this case get() result will depend on the 
>>> hash
>>> rather that on the inernal state of HashMap (such as HashMap length)
>>>   
>> I'm afraid the problem here is not the HashMap length,  whatever the 
>> length is, this kind of problem can happen, 16 just make it more 
>> easier, because it is not a prime number.
>>
>> So if my understanding is right, you suggest to cache the key's hash 
>> value as *original* hash into HashMap.Entry, when the key/value pair 
>> is put into HashMap; and when someone want to get back the value from 
>> HashMap by the same key, the cached *original* hash will be compared 
>> with the *current* hash, and if they are not equal, HashMap will 
>> consider them as non-match, even the keys are *same* object. In fact 
>> that's also my point of view to fix this non-compatibility. This fix 
>> is not difficult, but the only concern is, this behaviour may be 
>> controversial.
>>
>> thoughts?
>>> Thanks,
>>> Mikhail
>>>
>>> 2006/3/15, Paulex Yang <pa...@gmail.com>:
>>>  
>>>> Pls. try the test case on HashMap below.
>>>>
>>>> On RI, it print out:
>>>> null
>>>> null
>>>>
>>>> On Harmony, it print out
>>>> null
>>>> value1
>>>>
>>>> it is definitely bad practice to reuse a object as hash key by modify
>>>> its hashcode, but I DID see some similar cases before, after all, you
>>>> cannot force but only can *suggest* developers what to do.
>>>>
>>>> So, what should we do?  Try to replicate RI's behavior?
>>>>
>>>>
>>>> public class HashMapTest {
>>>>  public static void main(String[] args) {
>>>>  ReusableKey k = new ReusableKey();
>>>>  HashMap map = new HashMap();
>>>>  k.setKey(1);
>>>>  map.put(k, "value1");
>>>>
>>>>  k.setKey(18);
>>>>  //both RI and Harmony get null here
>>>>  System.out.println(map.get(k));
>>>>
>>>>  k.setKey(17);
>>>>  //RI get null
>>>>  //Harmony get "value1"
>>>>  //the number of 17 is *magic* because the default length of 
>>>> HashMap is 16.
>>>>  System.out.println(map.get(k));
>>>>  }
>>>> }
>>>>
>>>> class ReusableKey{
>>>>  private int key = 0;
>>>>  public void setKey(int key){
>>>>  this.key = key;
>>>>  }
>>>>  public int hashCode(){
>>>>  return key;
>>>>  }
>>>>  public boolean equals(Object o){
>>>>  if(o == this){
>>>>   return true;
>>>>  }
>>>>  if(!(o instanceof ReusableKey)){
>>>>   return false;
>>>>  }
>>>>  return key == ((ReusableKey)o).key;
>>>>  }
>>>> }
>>>>
>>>>
>>>> -- 
>>>> Paulex Yang
>>>> China Software Development Lab
>>>> IBM
>>>>
>>>>
>>>>
>>>>     
>>>
>>>   
>>
>>
>
>


-- 
Paulex Yang
China Software Development Lab
IBM



Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Mikhail Loenko <ml...@gmail.com>.
Hi Richard

2006/3/16, Richard Liang <ri...@gmail.com>:
> Hello Paulex,
>
> According to Java 5 Spec, "Note: great care must be exercised if mutable
> objects are used as map keys. The behavior of a map is not specified if
> the value of an object is changed in a manner that affects equals
> comparisons while the object is a key in the map...."


it is great that you have found this in the spec!


> As the behavior of a map is not specified, we can either keep compatible
> with RI, or leave it as is. But keeping compatible with RI may be a
> better solution. :-)

+1

Thanks,
Mikhail




>
> Paulex Yang wrote:
> > Hi, Mikhail,
> >
> > Mikhail Loenko wrote:
> >> Hi Paulex,
> >>
> >> IMHO from compatibility point of view any behavior is legal.
> >> HashMap is mostly designed for keys that do not change over time, at
> >> least
> >> spec silent about changing the keys. So, I think implementation
> >> specsific is
> >> allowed here.
> >>
> >> But I think that aiming stability we'd better compare current hash
> >> with the original one - in this case get() result will depend on the
> >> hash
> >> rather that on the inernal state of HashMap (such as HashMap length)
> >>
> > I'm afraid the problem here is not the HashMap length,  whatever the
> > length is, this kind of problem can happen, 16 just make it more
> > easier, because it is not a prime number.
> >
> > So if my understanding is right, you suggest to cache the key's hash
> > value as *original* hash into HashMap.Entry, when the key/value pair
> > is put into HashMap; and when someone want to get back the value from
> > HashMap by the same key, the cached *original* hash will be compared
> > with the *current* hash, and if they are not equal, HashMap will
> > consider them as non-match, even the keys are *same* object. In fact
> > that's also my point of view to fix this non-compatibility. This fix
> > is not difficult, but the only concern is, this behaviour may be
> > controversial.
> >
> > thoughts?
> >> Thanks,
> >> Mikhail
> >>
> >> 2006/3/15, Paulex Yang <pa...@gmail.com>:
> >>
> >>> Pls. try the test case on HashMap below.
> >>>
> >>> On RI, it print out:
> >>> null
> >>> null
> >>>
> >>> On Harmony, it print out
> >>> null
> >>> value1
> >>>
> >>> it is definitely bad practice to reuse a object as hash key by modify
> >>> its hashcode, but I DID see some similar cases before, after all, you
> >>> cannot force but only can *suggest* developers what to do.
> >>>
> >>> So, what should we do?  Try to replicate RI's behavior?
> >>>
> >>>
> >>> public class HashMapTest {
> >>>  public static void main(String[] args) {
> >>>  ReusableKey k = new ReusableKey();
> >>>  HashMap map = new HashMap();
> >>>  k.setKey(1);
> >>>  map.put(k, "value1");
> >>>
> >>>  k.setKey(18);
> >>>  //both RI and Harmony get null here
> >>>  System.out.println(map.get(k));
> >>>
> >>>  k.setKey(17);
> >>>  //RI get null
> >>>  //Harmony get "value1"
> >>>  //the number of 17 is *magic* because the default length of HashMap
> >>> is 16.
> >>>  System.out.println(map.get(k));
> >>>  }
> >>> }
> >>>
> >>> class ReusableKey{
> >>>  private int key = 0;
> >>>  public void setKey(int key){
> >>>  this.key = key;
> >>>  }
> >>>  public int hashCode(){
> >>>  return key;
> >>>  }
> >>>  public boolean equals(Object o){
> >>>  if(o == this){
> >>>   return true;
> >>>  }
> >>>  if(!(o instanceof ReusableKey)){
> >>>   return false;
> >>>  }
> >>>  return key == ((ReusableKey)o).key;
> >>>  }
> >>> }
> >>>
> >>>
> >>> --
> >>> Paulex Yang
> >>> China Software Development Lab
> >>> IBM
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
>
>
> --
> Richard Liang
> China Software Development Lab, IBM
>
>
>

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Richard Liang <ri...@gmail.com>.
Hello Paulex,

According to Java 5 Spec, "Note: great care must be exercised if mutable 
objects are used as map keys. The behavior of a map is not specified if 
the value of an object is changed in a manner that affects equals 
comparisons while the object is a key in the map...."

As the behavior of a map is not specified, we can either keep compatible 
with RI, or leave it as is. But keeping compatible with RI may be a 
better solution. :-)

Paulex Yang wrote:
> Hi, Mikhail,
>
> Mikhail Loenko wrote:
>> Hi Paulex,
>>
>> IMHO from compatibility point of view any behavior is legal.
>> HashMap is mostly designed for keys that do not change over time, at 
>> least
>> spec silent about changing the keys. So, I think implementation 
>> specsific is
>> allowed here.
>>
>> But I think that aiming stability we'd better compare current hash
>> with the original one - in this case get() result will depend on the 
>> hash
>> rather that on the inernal state of HashMap (such as HashMap length)
>>   
> I'm afraid the problem here is not the HashMap length,  whatever the 
> length is, this kind of problem can happen, 16 just make it more 
> easier, because it is not a prime number.
>
> So if my understanding is right, you suggest to cache the key's hash 
> value as *original* hash into HashMap.Entry, when the key/value pair 
> is put into HashMap; and when someone want to get back the value from 
> HashMap by the same key, the cached *original* hash will be compared 
> with the *current* hash, and if they are not equal, HashMap will 
> consider them as non-match, even the keys are *same* object. In fact 
> that's also my point of view to fix this non-compatibility. This fix 
> is not difficult, but the only concern is, this behaviour may be 
> controversial.
>
> thoughts?
>> Thanks,
>> Mikhail
>>
>> 2006/3/15, Paulex Yang <pa...@gmail.com>:
>>  
>>> Pls. try the test case on HashMap below.
>>>
>>> On RI, it print out:
>>> null
>>> null
>>>
>>> On Harmony, it print out
>>> null
>>> value1
>>>
>>> it is definitely bad practice to reuse a object as hash key by modify
>>> its hashcode, but I DID see some similar cases before, after all, you
>>> cannot force but only can *suggest* developers what to do.
>>>
>>> So, what should we do?  Try to replicate RI's behavior?
>>>
>>>
>>> public class HashMapTest {
>>>  public static void main(String[] args) {
>>>  ReusableKey k = new ReusableKey();
>>>  HashMap map = new HashMap();
>>>  k.setKey(1);
>>>  map.put(k, "value1");
>>>
>>>  k.setKey(18);
>>>  //both RI and Harmony get null here
>>>  System.out.println(map.get(k));
>>>
>>>  k.setKey(17);
>>>  //RI get null
>>>  //Harmony get "value1"
>>>  //the number of 17 is *magic* because the default length of HashMap 
>>> is 16.
>>>  System.out.println(map.get(k));
>>>  }
>>> }
>>>
>>> class ReusableKey{
>>>  private int key = 0;
>>>  public void setKey(int key){
>>>  this.key = key;
>>>  }
>>>  public int hashCode(){
>>>  return key;
>>>  }
>>>  public boolean equals(Object o){
>>>  if(o == this){
>>>   return true;
>>>  }
>>>  if(!(o instanceof ReusableKey)){
>>>   return false;
>>>  }
>>>  return key == ((ReusableKey)o).key;
>>>  }
>>> }
>>>
>>>
>>> -- 
>>> Paulex Yang
>>> China Software Development Lab
>>> IBM
>>>
>>>
>>>
>>>     
>>
>>   
>
>


-- 
Richard Liang
China Software Development Lab, IBM



Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Mikhail Loenko <ml...@gmail.com>.
Hi Paulex,

I meant exactly what you described: you need to compare *recorded* hash value
with current.key.hash().
(you also have to compare recorded.key == current.key)

This behavior would be at least consistent: result of getValue() will not depend
on hashmap length.

Why do you think it is controversial?

Thanks,
Mikhail


2006/3/16, Paulex Yang <pa...@gmail.com>:
> Hi, Mikhail,
>
> Mikhail Loenko wrote:
> > Hi Paulex,
> >
> > IMHO from compatibility point of view any behavior is legal.
> > HashMap is mostly designed for keys that do not change over time, at least
> > spec silent about changing the keys. So, I think implementation specsific is
> > allowed here.
> >
> > But I think that aiming stability we'd better compare current hash
> > with the original one - in this case get() result will depend on the hash
> > rather that on the inernal state of HashMap (such as HashMap length)
> >
> I'm afraid the problem here is not the HashMap length,  whatever the
> length is, this kind of problem can happen, 16 just make it more easier,
> because it is not a prime number.
>
> So if my understanding is right, you suggest to cache the key's hash
> value as *original* hash into HashMap.Entry, when the key/value pair is
> put into HashMap; and when someone want to get back the value from
> HashMap by the same key, the cached *original* hash will be compared
> with the *current* hash, and if they are not equal, HashMap will
> consider them as non-match, even the keys are *same* object. In fact
> that's also my point of view to fix this non-compatibility. This fix is
> not difficult, but the only concern is, this behaviour may be
> controversial.
>
> thoughts?
> > Thanks,
> > Mikhail
> >
> > 2006/3/15, Paulex Yang <pa...@gmail.com>:
> >
> >> Pls. try the test case on HashMap below.
> >>
> >> On RI, it print out:
> >> null
> >> null
> >>
> >> On Harmony, it print out
> >> null
> >> value1
> >>
> >> it is definitely bad practice to reuse a object as hash key by modify
> >> its hashcode, but I DID see some similar cases before, after all, you
> >> cannot force but only can *suggest* developers what to do.
> >>
> >> So, what should we do?  Try to replicate RI's behavior?
> >>
> >>
> >> public class HashMapTest {
> >>  public static void main(String[] args) {
> >>  ReusableKey k = new ReusableKey();
> >>  HashMap map = new HashMap();
> >>  k.setKey(1);
> >>  map.put(k, "value1");
> >>
> >>  k.setKey(18);
> >>  //both RI and Harmony get null here
> >>  System.out.println(map.get(k));
> >>
> >>  k.setKey(17);
> >>  //RI get null
> >>  //Harmony get "value1"
> >>  //the number of 17 is *magic* because the default length of HashMap is 16.
> >>  System.out.println(map.get(k));
> >>  }
> >> }
> >>
> >> class ReusableKey{
> >>  private int key = 0;
> >>  public void setKey(int key){
> >>  this.key = key;
> >>  }
> >>  public int hashCode(){
> >>  return key;
> >>  }
> >>  public boolean equals(Object o){
> >>  if(o == this){
> >>   return true;
> >>  }
> >>  if(!(o instanceof ReusableKey)){
> >>   return false;
> >>  }
> >>  return key == ((ReusableKey)o).key;
> >>  }
> >> }
> >>
> >>
> >> --
> >> Paulex Yang
> >> China Software Development Lab
> >> IBM
> >>
> >>
> >>
> >>
> >
> >
>
>
> --
> Paulex Yang
> China Software Development Lab
> IBM
>
>
>

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Paulex Yang <pa...@gmail.com>.
Hi, Mikhail,

Mikhail Loenko wrote:
> Hi Paulex,
>
> IMHO from compatibility point of view any behavior is legal.
> HashMap is mostly designed for keys that do not change over time, at least
> spec silent about changing the keys. So, I think implementation specsific is
> allowed here.
>
> But I think that aiming stability we'd better compare current hash
> with the original one - in this case get() result will depend on the hash
> rather that on the inernal state of HashMap (such as HashMap length)
>   
I'm afraid the problem here is not the HashMap length,  whatever the 
length is, this kind of problem can happen, 16 just make it more easier, 
because it is not a prime number.

So if my understanding is right, you suggest to cache the key's hash 
value as *original* hash into HashMap.Entry, when the key/value pair is 
put into HashMap; and when someone want to get back the value from 
HashMap by the same key, the cached *original* hash will be compared 
with the *current* hash, and if they are not equal, HashMap will 
consider them as non-match, even the keys are *same* object. In fact 
that's also my point of view to fix this non-compatibility. This fix is 
not difficult, but the only concern is, this behaviour may be 
controversial.

thoughts?
> Thanks,
> Mikhail
>
> 2006/3/15, Paulex Yang <pa...@gmail.com>:
>   
>> Pls. try the test case on HashMap below.
>>
>> On RI, it print out:
>> null
>> null
>>
>> On Harmony, it print out
>> null
>> value1
>>
>> it is definitely bad practice to reuse a object as hash key by modify
>> its hashcode, but I DID see some similar cases before, after all, you
>> cannot force but only can *suggest* developers what to do.
>>
>> So, what should we do?  Try to replicate RI's behavior?
>>
>>
>> public class HashMapTest {
>>  public static void main(String[] args) {
>>  ReusableKey k = new ReusableKey();
>>  HashMap map = new HashMap();
>>  k.setKey(1);
>>  map.put(k, "value1");
>>
>>  k.setKey(18);
>>  //both RI and Harmony get null here
>>  System.out.println(map.get(k));
>>
>>  k.setKey(17);
>>  //RI get null
>>  //Harmony get "value1"
>>  //the number of 17 is *magic* because the default length of HashMap is 16.
>>  System.out.println(map.get(k));
>>  }
>> }
>>
>> class ReusableKey{
>>  private int key = 0;
>>  public void setKey(int key){
>>  this.key = key;
>>  }
>>  public int hashCode(){
>>  return key;
>>  }
>>  public boolean equals(Object o){
>>  if(o == this){
>>   return true;
>>  }
>>  if(!(o instanceof ReusableKey)){
>>   return false;
>>  }
>>  return key == ((ReusableKey)o).key;
>>  }
>> }
>>
>>
>> --
>> Paulex Yang
>> China Software Development Lab
>> IBM
>>
>>
>>
>>     
>
>   


-- 
Paulex Yang
China Software Development Lab
IBM



Re: [classlib]bug-to-bug compatibility: HashMap

Posted by Mikhail Loenko <ml...@gmail.com>.
Hi Paulex,

IMHO from compatibility point of view any behavior is legal.
HashMap is mostly designed for keys that do not change over time, at least
spec silent about changing the keys. So, I think implementation specsific is
allowed here.

But I think that aiming stability we'd better compare current hash
with the original one - in this case get() result will depend on the hash
rather that on the inernal state of HashMap (such as HashMap length)

Thanks,
Mikhail

2006/3/15, Paulex Yang <pa...@gmail.com>:
> Pls. try the test case on HashMap below.
>
> On RI, it print out:
> null
> null
>
> On Harmony, it print out
> null
> value1
>
> it is definitely bad practice to reuse a object as hash key by modify
> its hashcode, but I DID see some similar cases before, after all, you
> cannot force but only can *suggest* developers what to do.
>
> So, what should we do?  Try to replicate RI's behavior?
>
>
> public class HashMapTest {
>  public static void main(String[] args) {
>  ReusableKey k = new ReusableKey();
>  HashMap map = new HashMap();
>  k.setKey(1);
>  map.put(k, "value1");
>
>  k.setKey(18);
>  //both RI and Harmony get null here
>  System.out.println(map.get(k));
>
>  k.setKey(17);
>  //RI get null
>  //Harmony get "value1"
>  //the number of 17 is *magic* because the default length of HashMap is 16.
>  System.out.println(map.get(k));
>  }
> }
>
> class ReusableKey{
>  private int key = 0;
>  public void setKey(int key){
>  this.key = key;
>  }
>  public int hashCode(){
>  return key;
>  }
>  public boolean equals(Object o){
>  if(o == this){
>   return true;
>  }
>  if(!(o instanceof ReusableKey)){
>   return false;
>  }
>  return key == ((ReusableKey)o).key;
>  }
> }
>
>
> --
> Paulex Yang
> China Software Development Lab
> IBM
>
>
>

Re: [classlib]bug-to-bug compatibility: HashMap

Posted by will pugh <wi...@sourcelabs.com>.
Will Sun's implementation ever find ReusableKey if the value has been 
changed?

It would not surprise me if this simple case Sun doesn't find the entry, 
because they do something like hashing the hash that ReusableKey 
returns, or only allocating a prime number of buckets, e.g. you ask for 
a size of 16 and get 17, because 16 is not prime.

This doesn't actually seems like crazy behaviour, since it is probably 
rather common to have programmers that are not very good at creating 
well distributed hashing algorithms, and so having something built in to 
help is probably pretty good design.

    --Will

Paulex Yang wrote:

> Pls. try the test case on HashMap below.
>
> On RI, it print out:
> null
> null
>
> On Harmony, it print out
> null
> value1
>
> it is definitely bad practice to reuse a object as hash key by modify 
> its hashcode, but I DID see some similar cases before, after all, you 
> cannot force but only can *suggest* developers what to do.
>
> So, what should we do?  Try to replicate RI's behavior?
>
>
> public class HashMapTest {
> public static void main(String[] args) {
>  ReusableKey k = new ReusableKey();
>  HashMap map = new HashMap();
>  k.setKey(1);
>  map.put(k, "value1");
>
>  k.setKey(18);
>  //both RI and Harmony get null here
>  System.out.println(map.get(k));
>
>  k.setKey(17);
>  //RI get null
>  //Harmony get "value1"
>  //the number of 17 is *magic* because the default length of HashMap 
> is 16.
>  System.out.println(map.get(k));
> }
> }
>
> class ReusableKey{
> private int key = 0;
> public void setKey(int key){
>  this.key = key;
> }
> public int hashCode(){
>  return key;
> }
> public boolean equals(Object o){
>  if(o == this){
>   return true;
>  }
>  if(!(o instanceof ReusableKey)){
>   return false;
>  }
>  return key == ((ReusableKey)o).key;
> }
> }
>
>