[ http://issues.apache.org/jira/browse/VELOCITY-453?page=comments#action_12433379 ] Alexey Panchenko commented on VELOCITY-453: -------------------------------------------
>Do you have hard numbers for that claim? Are these checks used on a hot path? >Just curious. I can claim without any numbers that code instantiating objects and doing reflection operations will be slower than the code which just compares the object fields. And yes, this code is on the hot path. ASTMethod calls it during template merge. I think everybody wants his template to merge quicker. > equals should at least do a if (o == this) return true; check. This short > cuts the equals() method in case you test against yourself (set insert e.g.) This object is never compared to itself. One first object is used as a key in the HashMap and new objects are instantiated every time to find the cached value. > I'd love to have unit tests for hashCode() and equals(). This class was in code base before my changes. How it comes it was added without unit tests? Just curious. :-) To be serious: It is inner class with package visibility. What package should the unit tests be in? > There is a potential NPE with params == null in hashCode() and equals() This object is never instantiated with params == null. I'd rather add "assert params != null" to the code (if 1.4 syntax is allowed in codebase). > [PATCH] Fix IntrospectionCacheData caching > ------------------------------------------ > > Key: VELOCITY-453 > URL: http://issues.apache.org/jira/browse/VELOCITY-453 > Project: Velocity > Issue Type: Bug > Components: Source > Affects Versions: 1.5 > Reporter: Alexey Panchenko > Assigned To: Will Glass-Husain > Fix For: 1.5 > > Attachments: ASTMethod_IntrospectionCacheData_cache, > ASTMethod_IntrospectionCacheData_cache-v2-retry.patch, > ASTMethod_IntrospectionCacheData_cache-v2.patch, > IntrospectionCacheDataTest-no-copyright.java, > IntrospectionCacheDataTest-no-copyright.java, > IntrospectionCacheDataTest-v4.java, IntrospectionCacheDataTest.java > > > The old code used Class[].hashCode() in MethodCacheKey.hashCode() > implementation. > hashCode() is not overriden for arrays so it returns different value for each > array instance. > The attached is the correct implementation and a test case to prove the > caching actually works. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
