Skip to content

Conversation

@alespour
Copy link
Contributor

@alespour alespour commented Apr 9, 2025

This PR makes handlers slice reused in router.matchAndDispatch in order to reduce slice (re)allocations. I think it is safe to reuse the slice since it is only used when order = true. Therefore, also added explicit if order check for iterating over handlers, although it is safe to keep iterating over nil slice when order = false, of course.

It is only a subtle optimization, but appeared when I profiled high-load scenario.

Benchmark with a single route:

before:

go test -v -run Benchmark_MatchAndDispatch -bench=. -benchmem -benchtime=10s

Benchmark_MatchAndDispatch-4   	13364338	       884.3 ns/op	     136 B/op	       3 allocs/op

after:

go test -v -run Benchmark_MatchAndDispatch -bench=. -benchmem -benchtime=10s

Benchmark_MatchAndDispatch-4   	14650513	       808.3 ns/op	     128 B/op	       2 allocs/op

I suppose with more routes, without reusing the slice, append would cause more re-allocations.

@alespour alespour closed this Apr 9, 2025
@alespour alespour reopened this Apr 9, 2025
@alespour alespour marked this pull request as ready for review April 9, 2025 13:00
@alespour alespour force-pushed the fix/reduce-route-dispatch-allocations branch from 067bbff to ee146f3 Compare April 10, 2025 14:38
@alespour alespour changed the title fix: reduce route dispatch allocations fix: reduce slice allocations in route dispatch Apr 10, 2025
@MattBrittan
Copy link
Contributor

Sorry for the long delay in responding to this, as you say it's a very subtile optimization but, I guess, the router forms part of the hot path so it's worth the slight decrease in readability. Thanks for the PR.

@MattBrittan MattBrittan merged commit 3c6d3cc into eclipse-paho:master Jul 20, 2025
1 check passed
algitbot pushed a commit to alpinelinux/build-server-status that referenced this pull request Nov 9, 2025
This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/eclipse/paho.mqtt.golang](https://github.com/eclipse/paho.mqtt.golang) | require | patch | `v1.5.0` -> `v1.5.1` |

---

### Release Notes

<details>
<summary>eclipse/paho.mqtt.golang (github.com/eclipse/paho.mqtt.golang)</summary>

### [`v1.5.1`](https://github.com/eclipse-paho/paho.mqtt.golang/releases/tag/v1.5.1)

[Compare Source](eclipse-paho/paho.mqtt.golang@v1.5.0...v1.5.1)

This is a minor release incorporating changes made in the 14 months since v1.5.0 (including updating dependencies, and raising the Go version to 1.24). The changes are relatively minor but address a potential security issue, possible panic, enable users to better monitor the connection status, and incorporate a few optimisations.

Thanks to those who have provided fixes/enhancements included in this release!

Special thanks to Paul Gerste at [Sonar](https://www.sonarsource.com/) for reporting issue [#&#8203;730](eclipse-paho/paho.mqtt.golang#730) via the Eclipse security team (fix was implemented in MR [#&#8203;714](eclipse-paho/paho.mqtt.golang#714) in May, github issue created just prior to this release). This issue arose where a topic > 65535 bytes was passed to the `Publish` function, due to the way the data was encoded the topic could leak into the message body. Please see issue [#&#8203;730](eclipse-paho/paho.mqtt.golang#730) for further details.

#### What's Changed

- Updating go dependencies from pub and sub into the containers before building by [@&#8203;JefJrFigueiredo](https://github.com/JefJrFigueiredo) in [eclipse-paho#691](eclipse-paho/paho.mqtt.golang#691)
- Optimize TCP connection logic by [@&#8203;geekeryy](https://github.com/geekeryy) in [eclipse-paho#713](eclipse-paho/paho.mqtt.golang#713)
- Fields over 65535 bytes not encoded correctly by [@&#8203;MattBrittan](https://github.com/MattBrittan) in [eclipse-paho#714](eclipse-paho/paho.mqtt.golang#714)
- Reduce slice allocations in route dispatch  by [@&#8203;alespour](https://github.com/alespour) in [eclipse-paho#710](eclipse-paho/paho.mqtt.golang#710)
- Add a ConnectionNotificationHandler by [@&#8203;RangelReale](https://github.com/RangelReale) in [eclipse-paho#727](eclipse-paho/paho.mqtt.golang#727)
- Potential panic when using manual ACK by [@&#8203;MattBrittan](https://github.com/MattBrittan) in [eclipse-paho#729](eclipse-paho/paho.mqtt.golang#729)

**Full Changelog**: <eclipse-paho/paho.mqtt.golang@v1.5.0...v1.5.1>

</details>

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

&nbsp;
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNzMuMSIsInVwZGF0ZWRJblZlciI6IjQxLjE3My4xIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

See merge request alpine/infra/build-server-status!22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants