mamoe / mirai

高效率 QQ 机器人支持库
https://mirai.mamoe.net
GNU Affero General Public License v3.0
14.48k stars 2.54k forks source link

`Base64` 的 JVM 实现应当避免反复创建实例 #2288

Closed Colerar closed 2 years ago

Colerar commented 2 years ago

应将:

Ref

public actual fun ByteArray.encodeBase64(): String {
    return Base64.getEncoder().encodeToString(this)
}

public actual fun String.decodeBase64(): ByteArray {
    return Base64.getDecoder().decode(this)
}

改为:

private val base64Encoder = Base64.getEncoder()

private val base64Decoder = Base64.getDecoder()

public actual fun ByteArray.encodeBase64(): String {
    return base64Encoder.encodeToString(this)
}

public actual fun String.decodeBase64(): ByteArray {
    return base64Decoder.decode(this)
}

This class consists exclusively of static methods for obtaining encoders and decoders for the Base64 encoding scheme. The implementation of this class supports the following types of Base64 as specified in RFC 4648 and RFC 2045 .

Karlatemp commented 2 years ago

What are you meaning?

Karlatemp commented 2 years ago

为什么不看往未来 而是要停留在过去

sandtechnology commented 2 years ago

这玩意本来就是静态返回常量的 难道说你要论证一下单条INVOKENSTATIC指令对mirai运行的影响吗( 就算你要论证 如果方法调用频繁 会由JVM动态进行内联的 这种优化反而会带来可维护性和可读性的降低 属于没必要的优化 另外优化应该优化热点方法,做优化请带上二八定律的想法,把精力放到最有用的事情上,做完优化做下修改前后的benchmark,免得造成负优化(

Colerar commented 2 years ago

这玩意本来就是静态返回常量的 难道说你要论证一下单条INVOKENSTATIC指令对mirai运行的影响吗(

关于这点确实是我没看清楚。

这种优化反而会带来可维护性和可读性的降低 属于没必要的优化

Mirai 里面 @JvmStatic 还见得少吗?这是否属于「带来可维护性和可读性的降低 属于没必要的优化」。

另外优化应该优化热点方法,做优化请带上二八定律的想法,把精力放到最有用的事情上 这种优化反而会带来可维护性和可读性的降低 属于没必要的优化

你没有资格指责开源贡献者的精力所向,就算我每天开一百个 issue 修复或者甚至修复歪了,那也是我的乐趣所在。至于主分支是否愿意合并,那是 Maintainer 该考虑的。

另:Mirai 里面不可读的代码多了去了,就比如这个三连 .also

做完优化做下修改前后的benchmark,免得造成负优化

这是一个 issue,而不是 PR。在 benchmark 前先发个 issue prototype 是很正常的。不必在这点上纠结。

Him188 commented 2 years ago

@JvmStatic 是必要的,它是 Kotlin 现阶段没有 namespace 等概念为类型定义静态方法的 workaround。在 Java 使用 Image.Companion.fromId 是不如 Image.fromId 的。

JvmStatic 并不是优化,它会生成额外的静态方法而并不会删除成员方法。