On Thu, 27 Jan 2022 22:06:21 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
> This update is to support signature schemes customization for individual > (D)TLS connection. Please review the CSR as well: > CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 > RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 I like the per-parameters configuration and we really should get it for more Systeme properties in the future. But I wonder if the additional allocations (some while doing handshakes?) can be avoided by doing more parsing/analysing in the parameter object already. It could even do the analysis of the system property at that time. Also the toggle semantic seems to be a bit different then you would expect, a extra flag to keep track of customized settings would probably fix that and avoid the array compares. src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 92: > 90: private int maximumPacketSize = 0; > 91: private String[] applicationProtocols = new String[0]; > 92: private String[] signatureSchemes = new String[0]; There are some places in here which could use a local or global static EMPTY = new String[0]` value. That would be safe to share and reduces allocations (they surely escape). src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 745: > 743: * This method will make a copy of the {@code signatureSchemes} > array. > 744: * > 745: * @param signatureSchemes an ordered array of signature scheme > names, Should it mention that unknown schemes are ignored and not trigger a exception? src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 763: > 761: > 762: String[] tempSchemes = signatureSchemes.clone(); > 763: for (String scheme : tempSchemes) { In addition to this loop we could also parse/decompose the strings. This would do the work only once (if the parameter is reused) src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 66: > 64: // The configured signature schemes for "signature_algorithms" and > 65: // "signature_algorithms_cert" extensions > 66: String[] signatureSchemes; I would keep the typed list (get it from the parameters or system property cache). src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 262: > 260: } // otherwise, use the default values > 261: > 262: String[] ss = params.getSignatureSchemes(); If we don’t use the cloning getter here (and use the pre-parsed list) it would be more efficient. src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 410: > 408: > 409: // Reset the signature schemes, if it was configured with > SSLParameters. > 410: if (Arrays.equals(signatureSchemes, The duals here does not mean it was configure with parameters, it only means it is currently the same configuration as the parameters, but it still could be a different source? src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 439: > 437: (config.signatureSchemes == null || > 438: config.signatureSchemes.length == 0 || > 439: Arrays.asList(config.signatureSchemes) Is that a allocation per handshake*availablesize? ------------- PR: https://git.openjdk.java.net/jdk/pull/7252
