Fall back properly if chosen factory class isn't working. #28
Conversation
…sen one isn't working. Don't bother benchmarking the chosen factory class; it either works or it doesn't.
|
Note: this is the important parts of #26, without the PKCS11 change which is much less urgent. |
| // unnecessary. Technically though, the specified CipherFactory may | ||
| // malfunction. That is why FACTORY_CLASS_NAME is selected after it has | ||
| // proven itself functional. | ||
| boolean chosenFactoryClass = (factory.getClass().equals(Aes.factoryClass)); |
There was a problem hiding this comment.
Ah, no, I reorganized this from an if statement.
|
|
||
| if (chosenFactoryClass) | ||
| { | ||
| if (minFactory != null) |
There was a problem hiding this comment.
I'm not sure I understand this well. If this loop iteration is for the user-configured factory class, return the fastest factory. If a previous non-user-configured factory was faster, we short-circuit and return it (the faster one)?
Should that return factory instead of minFactory?
There was a problem hiding this comment.
Well, elsewhere in the code I reorganize the list so the user-configured factory class is always first, so we avoid profiling factories we don't need to. But I agree that's fragile and confusing.
I've reorganized it now, is it clearer?
| { | ||
| benchmark.run(cipher); | ||
| } | ||
| } | ||
|
|
||
| long startTime = System.nanoTime(); | ||
| for (int i = 0; i < NUM_BENCHMARKS; i++) | ||
| for (int i = 0; i < numBenchmarks; i++) |
There was a problem hiding this comment.
Should line 329 use numBenchmarks instead of NUM_BENCHMARKS? I guess in practice it doesn't matter
| { | ||
| for (Class<?> clazz : factoryClasses) | ||
| Class<?>[] newFactoryClasses; | ||
| if (add) |
There was a problem hiding this comment.
I think all of this is equivalent to factoryClasses = ArrayUtils.add(factoryClasses, Class.class, factoryClass); (from jitsi-utils)
There was a problem hiding this comment.
Not quite - I want to put the new factory class first, whereas ArrayUtils.add would seem to put it last.
Don't bother benchmarking the chosen factory class; it either works or it doesn't.
Also includes a few minor other cleanups.