Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read the clock fewer times during message routing #408

Merged

Conversation

dferstay
Copy link
Contributor

@dferstay dferstay commented Dec 2, 2020

Previously, we read the system clock twice for each event unless we
switched partitions early due to reaching maxBatchingMessages or
maxBatchingSize.

Now, we read the clock once every maxBatchingMessages / 100 messages.

This improves performance (especially for larger batch sizes) as the
router function is called for every message produced.

A bench test of the default router was added; results are below:

name             old time/op    new time/op    delta
DefaultRouter       106ns ± 0%      16ns ± 1%  -84.94%  (p=0.016 n=4+5)
DefaultRouter-4     106ns ± 0%      16ns ± 0%     ~     (p=0.079 n=4+5)

Signed-off-by: Daniel Ferstay dferstay@splunk.com

Motivation

The default_router reads the system clock twice for every message routed in the production code path. When testing at scale with larger batch sizes the amount of time spent reading the system clock during message routing shows up in pprof profiles.

Modifications

This change modifies the default router to read the system clock once for every maxBatchingMessages / 100 messages routed.
A bench test of the default router was added to verify that the change yields a speedup.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no

Documentation

  • Does this pull request introduce a new feature? no

@dferstay dferstay force-pushed the fewer-calls-to-time-during-routing branch 2 times, most recently from 3105115 to a398260 Compare December 2, 2020 05:34
@wolfstudy wolfstudy added this to the 0.4.0 milestone Dec 2, 2020
@wolfstudy
Copy link
Member

@dferstay Can you fix the ci error?

Previously, we read the system clock twice for each event unless we
switched partitions early due to reaching `maxBatchingMessages` or
`maxBatchingSize`.

Now, we read the clock once every `maxBatchingMessages / 100` messages.

This improves performance (especially for larger batch sizes) as the
router function is called for every message produced.

A bench test of the default router was added; results are below:
```
name             old time/op    new time/op    delta
DefaultRouter       106ns ± 0%      16ns ± 1%  -84.94%  (p=0.016 n=4+5)
DefaultRouter-4     106ns ± 0%      16ns ± 0%     ~     (p=0.079 n=4+5)
```

Signed-off-by: Daniel Ferstay <dferstay@splunk.com>
@dferstay dferstay force-pushed the fewer-calls-to-time-during-routing branch from a398260 to f32709f Compare December 2, 2020 06:04
@wolfstudy wolfstudy self-requested a review December 2, 2020 06:44
@wolfstudy wolfstudy merged commit 9fe66ed into apache:master Dec 2, 2020
tisonkun pushed a commit to tisonkun/pulsar-client-go that referenced this pull request Aug 15, 2023
…ache#408)

* Add command topic Deduplication(streamnative/pulsar-admin-go#246)

- pulsarctl topics get-deduplication [topic]
- pulsarctl topics set-deduplication [topic] -e
- pulsarctl topics remove-deduplication [topic]

* Modify prompt

Signed-off-by: limingnihao <limingnihao@live.com>
tisonkun pushed a commit to tisonkun/pulsar-client-go that referenced this pull request Aug 15, 2023
* Add command topic Deduplication(streamnative/pulsarctl#246)

- pulsarctl topics get-deduplication [topic]
- pulsarctl topics set-deduplication [topic] -e
- pulsarctl topics remove-deduplication [topic]

* Modify prompt

Signed-off-by: limingnihao <limingnihao@live.com>
tisonkun pushed a commit that referenced this pull request Aug 16, 2023
* Add command topic Deduplication(streamnative/pulsarctl#246)

- pulsarctl topics get-deduplication [topic]
- pulsarctl topics set-deduplication [topic] -e
- pulsarctl topics remove-deduplication [topic]

* Modify prompt

Signed-off-by: limingnihao <limingnihao@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants