🚀 [Feature]: Allow rebuilding route tree stack

[rebaz94]

Feature Description

I was thinking it'd be awesome if the buildTree method in App could be made public. This tweak would open up possibilities for dynamic request handling.

already checked #735 I get that rebuilding the tree might be resource-intensive, but we'd only use it during development. This way, we can ensure our routing behaves as expected without resorting to an * route and redoing all the routing and parsing logic.

Our goal is to enable users to define routes dynamically, with handlers loaded in on-the-fly. So, by simply making the buildTree method public, it'd really help us out.

Additional Context (optional)

The code snippet below illustrates how making buildTree public could streamline fiber routing without the need for using *.

s.app.Get("/define", func(ctx *fiber.Ctx) error {
		// load config
		path := "/test/:param"
		s.app.Get(path, func(ctx *fiber.Ctx) error {
			// load the handler instruction
			// executing logic written by user
			// can access the :param easily
			return nil
		})
		// after each definition
		s.app.BuildTree()

		return ctx.JSON(map[string]any{
			"success": true,
		})
	})

Code Snippet (optional)

No response

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have checked for existing issues that describe my suggestion prior to opening this one.
• I understand that improperly formatted feature requests may be closed without explanation.

[welcome]

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

we can allow this as a feature in version 3
we just have to note that we have to check the internal methods again and secure them via mutex, otherwise race conditions will occur
which has not played a role so far because the registration usually happened sequentially at the beginning of the project configuration

[luk3skyw4lker]

I can check this out! I'll be making sure that the method can be thread-safe and make a PR

[efectn]

I can check this out! I'll be making sure that the method can be thread-safe and make a PR

You can check https://github.com/gofiber/fiber/tree/export-buildtree branch. I've implemented the necessary mutexes to make the feature threas-safe but it affected the results of parallel Next benchmarks.

without mutex:
Benchmark_Router_Next_Parallel-16 15807055 82.79 ns/op 7 B/op 0 allocs/op

with mutex:
Benchmark_Router_Next_Parallel-16 6951328 162.0 ns/op 9 B/op 0 allocs/op

with rwmutex:
Benchmark_Router_Next_Parallel-16 12640818 94.48 ns/op 26 B/op 0 allocs/op

[gaby]

@efectn To avoid the Mutex we would need a separate Tree that's a copy of the main one. We can then sync changes to that one if the User generates dynamic routes?

• Start App
• Create Tree (one used by Next()
• Make copy of this tree after creation
• Sync changes to copy?

[efectn]

@efectn To avoid the Mutex we would need a separate Tree that's a copy of the main one. We can then sync changes to that one if the User generates dynamic routes?

• Start App
• Create Tree (one used by Next()
• Make copy of this tree after creation
• Sync changes to copy?

Wouldn't it complicate the Next since we are going to have 2 route tree?

[luk3skyw4lker]

It would probably still need mutexes to sync the changes no? Because if you're trying to rebuild it in runtime you would run the risk of reading or writing twice into the actual route tree. Like if you call the BuildTree method in two handlers that receive a request at the same time.

If the increase in the benchmarks are really unnaceptable, I don't know if this should go ahead, it will for sure increase the execution time inevitably. I'm a recent contributor, so I may be wrong about this, but from what I know this would happen eventually.

[gaby]

@luk3skyw4lker Yeah, I was reading the ticket wrong 😂 We had a discussion about it on Discord. Adding this def makes Fiber slower.

I think the idea may be to add it without the mutex but a big warning to the user, right? @efectn

Only a mutex in the public BuildTree() to avoid multiple calls to it, since its an expensive function.

[luk3skyw4lker]

@gaby happens to the best of us 😆 I should probably pay more attention at the Discord too tho, would love to be part of this discussion.

I wouldn't be a big fan of adding it without the mutexes but as I said, if the benchmark increases are in fact really unnaceptable, it would definitely be the best way to go.

[luk3skyw4lker]

@efectn @gaby I don't know if either of you are doing some work into this, but I've added a RebuildTree method with an extensive comment explaining all the reasons on why the user shouldn't use this method lightly and will add some docs on it and make a PR.

Would that be cool?

[efectn]

@efectn @gaby I don't know if either of you are doing some work into this, but I've added a RebuildTree method with an extensive comment explaining all the reasons on why the user shouldn't use this method lightly and will add some docs on it and make a PR.

Would that be cool?

Feel free to create a PR. I've been also planning to return params on RoutePatternMatch so as to make it safe alternative to dynamic route registration.