Another concern of mine is about that the returned MessageDigest object heavily depends on the providers configuration when the object get instantiated for the 1st time. If the providers configuration get updated during run time, the provider of the returned MessageDigest object does not get updated accordingly. This proposal may be a pretty good practice in application layer in some circumstance, but may result in unnecessary complexity in JRE layer because we don't know the exact context that the application want to use this class, and the exact implementation details of the underlying MD service provider.
On 5/6/2014 5:45 PM, Florian Weimer wrote: > On 05/05/2014 04:43 PM, Xuelei Fan wrote: > >>> It's 10% faster, even including the digest overhead for a >>> single-block message. > >> clone() is an optional operation for MD. This point may make this >> class unreliable. > > I think the MessageDigest specification requires that this works. Per the spec, clone() may throw CloneNotSupportedException. It is OK a certain provider does not support Cloneable. > The > TLS implementation also relies on this behavior (i.e. clone() either > consistently fails or consistently succeeds). > That's true. SunJSSE requires cloneable MD implementation. > Note that I'm not blindly calling clone(), I'm providing a fallback in > case clone() does not work. > It's a good workaround. >> I think it might be a better place to make the improvement in crypto >> providers. > > What kind of improvement are you thinking of? > If the performance can get improvement here by define a static cloneable object, it might be also can get improvement in the same way in the provider implementation, unless the improvement is all about implementation service look up among the supported providers. >> BTW! I guess in some situation or some providers, clone() might not be >> a lightweight operation. > > Hmm. I can see that the state cannot be cloned at all (that's why > cloning is optional), but allocating a new state has to happen anyway, > no matter how the object is constructed. > Let's consider a case, every MD object should be bound to a session, and the operations should be synchronized in the session. clone() will share the session, the operations among different MD objects that share the session need to be synchronized. I think, there is a significant performance and scalablity impact if StandardMessageDigests is used for such cases in concurrency context. >>> I think the single-block hashing case is important because it is not >>> always possible to reuse an existing digest object after calling >>> reset(). > >> It means the reset() does not work. Looks like a bug to me. > > What I meant is that there might be no place to store the reference to a > long-term MessageDigest object which is reused again and again, so the > best thing the code can do is to allocate a fresh object when it is needed? > The object of MessageDigest is not immutable. IMHO, as we discussed above, there are a lot of issues need to be considered carefully to support such a long-term object in JRE. Thanks, Xuelei > Hmm, maybe we could provide a tread-local message digest that can be > used to hash byte arrays? It would address the simplest possible case > where the instantiation overhead is most visible. >
