harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From oleg babul <oleg...@gmail.com>
Subject Re: [classlib][luni] HashMap doesn't support proxy object as keys
Date Wed, 08 Jul 2009 07:21:47 GMT
To find entry in the HashMap RI performs:
1) Check that entry.hash==key.hashCode() (entry - hash map entry)
2) Check that entry.key == key or (key!=null && key.eqauls(entry.key))

hm.get(proxyInstance); returns !=null object because entry.key == key
in the 2) and equals doesn't perform.

Oleg

On Wed, Jul 8, 2009 at 8:34 AM, Regis<xu.regis@gmail.com> wrote:
> Jim Yu wrote:
>>
>> 2009/6/18 Regis <xu.regis@gmail.com>
>>
>>> Jim Yu wrote:
>>>
>>>> 2009/6/18 Regis <xu.regis@gmail.com>
>>>>
>>>>  Jim Yu wrote:
>>>>>
>>>>>  Hi all,
>>>>>>
>>>>>> There is an interesting edge case in HashMap. If we use a proxy object
>>>>>> as
>>>>>> the key to put something into HashMap, we will fail to retrieve the
>>>>>> value
>>>>>> by
>>>>>> using that key. But RI works well for this case. Here is a test case
>>>>>> below
>>>>>> to present the problem. I found the root cause of the failure for
our
>>>>>> HashMap is that proxyInstance.equals(proxyInstance) returns false
>>>>>> which
>>>>>> sounds strange but appears work correctly as both Harmony and RI
>>>>>> behave
>>>>>> so.
>>>>>>
>>>>>>  It's very interesting behaviors, seems like RI did it intended,
are
>>>>>
>>>>> there
>>>>> any cases we need proxyInstance.equals(proxyInstance) return false?
>>>>>
>>>>
>>>> I think it is reasonable to return false here since spec mentioned for
>>>> proxy
>>>> instance that "An invocation of the hashCode, equals, or toString
>>>> methods
>>>> declared in java.lang.Object on a proxy instance will be encoded and
>>>> dispatched to the invocation handler's invoke method" So the meaning of
>>>> equals method seems has been changed by the invoke here: )
>>>>
>>>>
>>>>>  I suspect RI has made some special approaches to match the key when
>>>>> the
>>>>>
>>>>>> key
>>>>>> is a proxy object. So I would be inclined to follow RI's behavior
in
>>>>>> this
>>>>>> case. Any thoughts here?
>>>>>>
>>>>>>  I think if proxyInstance.equals(proxyInstance) return false is
>>>>>
>>>>> reasonable,
>>>>> we should do some tricks to make HashMap work with Proxy. And does
>>>>> Proxy
>>>>> object work well with other collections which used equals to retrieve
>>>>> object
>>>>> from collection?
>>>>>
>>>>>
>>>> Agree. I have made a patch to do the trick so as to follow RI's
>>>> behavior.
>>>>
>>> The patch looks good for me, applied at r786015, please verify.
>>
>>
>> Verified at r786379.  Thanks, Regis.
>>
>>>
>>>
>>>>
>>>>>  I have raised a JIRA at
>>>>>>
>>>>>> https://issues.apache.org/jira/browse/HARMONY-6237for this issue.
>>>>>>
>>>>>>
>>>>>> public interface MockInterface {
>>>>>>  public String mockMethod();
>>>>>> }
>>>>>>
>>>>>> public class MockClass implements MockInterface {
>>>>>>  public String mockMethod() {
>>>>>>      return "This is a mock class.";
>>>>>>  }
>>>>>> }
>>>>>>
>>>>>> import java.lang.reflect.InvocationHandler;
>>>>>> import java.lang.reflect.Method;
>>>>>> import java.lang.reflect.Proxy;
>>>>>> import java.util.HashMap;
>>>>>> import java.util.Map;
>>>>>>
>>>>>> public class TestProxy implements InvocationHandler {
>>>>>>
>>>>>>  Object obj;
>>>>>>
>>>>>>  public TestProxy(Object o) {
>>>>>>      obj = o;
>>>>>>  }
>>>>>>
>>>>>>  public Object invoke(Object proxy, Method m, Object[] args)
>>>>>>          throws Throwable {
>>>>>>
>>>>>>      Object result = null;
>>>>>>
>>>>>>      try {
>>>>>>
>>>>>>          result = m.invoke(obj, args);
>>>>>>
>>>>>>      } catch (Exception e) {
>>>>>>          e.printStackTrace();
>>>>>>      } finally {
>>>>>>      }
>>>>>>      return result;
>>>>>>  }
>>>>>>
>>>>>>  public static void main(String[] argv) throws Exception {
>>>>>>
>>>>>>      MockInterface proxyInstance = (MockInterface)
>>>>>> Proxy.newProxyInstance(
>>>>>>              MockInterface.class.getClassLoader(),
>>>>>>              new Class[] { MockInterface.class }, new TestProxy(
>>>>>>                      new MockClass()));
>>>>>>
>>>>>>      Map hm = new HashMap();
>>>>>>
>>>>>>      hm.put(proxyInstance, "Value");
>>>>>>
>>>>>>      Object o = hm.get(proxyInstance);
>>>>>>
>>>>>>      System.out.println("Value got for proxy object key:" + o);
>>>>>>
>>>>>>      System.out.println(proxyInstance.equals(proxyInstance));
>>>>>>
>>>>>>  }
>>>>>> }
>>>>>>
>>>>>> Output
>>>>>> Harmony:
>>>>>> Value got for proxy object key:null
>>>>>> false
>>>>>>
>>>>>> RI:
>>>>>> Value got for proxy object key:Value
>>>>>> false
>>>>>>
>>>>>>
>>>>>>  --
>>>>>
>>>>> Best Regards,
>>>>> Regis.
>>>>>
>>>>>
>>>>
>>>>
>>> --
>>> Best Regards,
>>> Regis.
>>>
>>
>>
>>
>
> I found this patch involve serious performance regression. Because
> Proxy.isProxyClass() synchronized on a instance "proxyCache", which is a
> static field of Proxy, so all HashMap.get() will contend for this lock. So
> is there other way to test whether a class is Proxy class or
> Proxy.isProxyClass() could avoid to use lock of static field?
>
>
> --
> Best Regards,
> Regis.
>

Mime
View raw message