[improve][client] Refactor SchemaHash to reduce call of hashFunction in SchemaHash

[AnonHxy]

Fixes #17931

Motivation

The PR #17049 bring a significant performance regression for publish throughput. The root cause is that HashFunction#hashBytes(byte[]) takes a lot of cpu time. So we need add a cache for SchemaHash.

For details see line97:

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageImpl.java

Lines 85 to 99 in 8d13ff8

	public static <T> MessageImpl<T> create(MessageMetadata msgMetadata, ByteBuffer payload, Schema<T> schema,
	String topic) {
	@SuppressWarnings("unchecked")
	MessageImpl<T> msg = (MessageImpl<T>) RECYCLER.get();
	msg.msgMetadata.clear();
	msg.msgMetadata.copyFrom(msgMetadata);
	msg.messageId = null;
	msg.topic = topic;
	msg.cnx = null;
	msg.payload = Unpooled.wrappedBuffer(payload);
	msg.properties = null;
	msg.schema = schema;
	msg.schemaHash = SchemaHash.of(schema);
	msg.uncompressedSize = payload.remaining();
	return msg;

And we can see that hashFunction takes a lot of cpu time from the flame graph below

The test result for publish throughput(msgs/s) is below：

	master branch	master branch revert PR#17049	master branch apply this patch
1	837,119.26	1,504,240.76	1,570,375.90
2	870,065.55	1,504,240.76	1,594,203.19
3	868,281.75	1,504,240.76	1,589,468.89
avg	858,488.85	1,528,647.63	1,584,682.66

Modifications

• Adding a cache for SchemaHash.
• To avoid the new byte[0] allocation and instead passing a null for SchemaHash#of

Verifying this change

• Make sure that the change passes the CI checks.

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: AnonHxy#8

[merlimat]

One more change could be:

 public SchemaHash getSchemaHash() {
        return schemaHash == null ? SchemaHash.of(new byte[0], null) : schemaHash;
    }

to avoid the new byte[0] allocation and instead passing a null;

[AnonHxy]

to avoid the new byte[0] allocation and instead passing a null;

A great suggestion

[AnonHxy]

@Jason918 @merlimat @codelipenghui @Technoboy- PTAL

[codelipenghui]

@merlimat Please help review this PR again.

[eolivelli]

I approve this patch but we cannot chery pick to released branches.
We are changing the internal API of SchemaInfoImpl.
Unfortunately Pulsar users are very creative and they could have used it.

If we find a way to not break the internal builder API and make it compatible we can cherry pick

[codelipenghui]

@eolivelli I think this PR will not break the SchemaInfoImpl API. It just added SchemaHash to the SchemaInfoImpl . Do you see any point that it will break the builder API?

The old API like the followings are still available for users.

SCHEMA_INFO = new SchemaInfoImpl()
        .setName("Boolean")
        .setType(SchemaType.BOOLEAN)
        .setSchema(new byte[0]);

[AnonHxy]

If we find a way to not break the internal builder API and make it compatible we can cherry pick

If users create SchemaInfo by no-args-constructor, the SchemaHash in SchemaInfoImpl will not be initialized, and SchemaHash#of(SchemaInfo) will return null.

If the compatible issues you mentioned is that, I think we can upated SchemaInfoImpl#getSchemaHash like below: WDYT @codelipenghui @eolivelli

 public SchemaHash getSchemaHash() {
        if (schemaHash == null) {
            schemaHash = SchemaHash.of(this.schema, this.type);
        }
        return schemaHash;
    }

[eolivelli]

LGTM

great work

[AnonHxy]

/pulsarbot run-failure-checks

[nicoloboschi]

LGTM

[nicoloboschi]

@AnonHxy please update the pr title to make the semantic check to pass. [improve][client] should be accepted
I've restarted the failed jobs

[Jason918]

@AnonHxy please update the pr title to make the semantic check to pass. [improve][client] should be accepted
I've restarted the failed jobs

The title check seems don't accept the "[issue-xxx]" part.