Query, Header Default Value Tag Feature

[muratmirgun]

Description

This Feature provides an approach in Go to assign default values to the fields of structs. The tagHandlers function assigns these default values based on the specific type of a field. To enhance performance, we cache which fields of the structs have default values (structCache), so we don't have to repeatedly look up these fields every time. Because Handlers Query structs not change

Fixes:
#2571

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

Checklist:

• For new functionalities I follow the inspiration of the express js framework and built them similar in usage
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation - /docs/ directory for https://docs.gofiber.io/
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• If new dependencies exist, I have checked that they are really necessary and agreed with the maintainers/community (we want to have as few dependencies as possible)
• I tried to make my code as fast as possible with as few allocations as possible
• For new code I have written benchmarks so that they can be analyzed and improved

Commit formatting:

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

[welcome]

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

Thx

Can you share the benchmark results
How big is the impact for cases without defaults, do we have pay for that as well?

Can you put the default value handling and the tests in a separate go file (don't like big files, I'm more of a code divider)?
If we have this built in for queryparser, it would probably make sense to build it in for the other parsers (body & header) as well.

Should we add a config setting so you can disable this handling to get the maximum performance if you don't need defaults or have your own handling?

[muratmirgun]

Thx

Can you share the benchmark results How big is the impact for cases without defaults, do we have pay for that as well?

Can you put the default value handling and the tests in a separate go file (don't like big files, I'm more of a code divider)? If we have this built in for queryparser, it would probably make sense to build it in for the other parsers (body & header) as well.

Should we add a config setting so you can disable this handling to get the maximum performance if you don't need defaults or have your own handling?

First, let me share the benchmark results. but These examples for ColdStart

Benchmark_Ctx_QueryParser
Benchmark_Ctx_QueryParser-8 653192 1621 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 751180 1611 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 753249 1563 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 771404 1646 ns/op 856 B/op 38 allocs/op

Benchmark_Ctx_QueryParserWithDefaultValues
Benchmark_Ctx_QueryParserWithDefaultValues-8 974080 1239 ns/op 720 B/op 25 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 941806 1245 ns/op 720 B/op 25 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 963400 1220 ns/op 720 B/op 25 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 963571 1236 ns/op 720 B/op 25 allocs/op

Benchmarks for multiple Request Scenario:

Benchmark_Ctx_QueryParser
Benchmark_Ctx_QueryParser-8 14372 78933 ns/op 42817 B/op 1900 allocs/op
Benchmark_Ctx_QueryParser-8 15265 77700 ns/op 42817 B/op 1900 allocs/op
Benchmark_Ctx_QueryParser-8 15346 78469 ns/op 42817 B/op 1900 allocs/op
Benchmark_Ctx_QueryParser-8 15432 78051 ns/op 42817 B/op 1900 allocs/op

Benchmark_Ctx_QueryParserWithDefaultValues
Benchmark_Ctx_QueryParserWithDefaultValues-8 19756 60628 ns/op 36018 B/op 1250 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 19581 60628 ns/op 36018 B/op 1250 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 19682 60993 ns/op 36017 B/op 1250 allocs/op
Benchmark_Ctx_QueryParserWithDefaultValues-8 19599 60733 ns/op 36018 B/op 1250 allocs/op

With multiple

Apart from that, we can do it for all parsers, it will be logical and we can do them with a separate file logic. In the cold start logic, there is a small cost involuntarily at first until the first cache settles, but there will be no problem after the request comes to all the handlers that contain the default structure.

If we are going to integrate it into all parse structures, we can control this with the config for users who are worried about speed. If we think about the structure we're talking about, I can do that.

[ReneWerner87]

Can you share the performance without and with your function for the queryparser (i.e. the times from before vs. after the adjustment)

[muratmirgun]

Can you share the performance without and with your function for the queryparser (i.e. the times from before vs. after the adjustment)

Sorry, I forgot it. You can check in your computer from my fork

Benchmark_Ctx_QueryParser
Benchmark_Ctx_QueryParser-8 679710 1629 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 740930 1625 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 752588 1584 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser-8 763689 1578 ns/op 856 B/op 38 allocs/op
Benchmark_Ctx_QueryParser_Comma
Benchmark_Ctx_QueryParser_Comma-8 665384 1764 ns/op 928 B/op 44 allocs/op
Benchmark_Ctx_QueryParser_Comma-8 690904 1766 ns/op 928 B/op 44 allocs/op
Benchmark_Ctx_QueryParser_Comma-8 678898 1767 ns/op 928 B/op 44 allocs/op
Benchmark_Ctx_QueryParser_Comma-8 683109 1754 ns/op 928 B/op 44 allocs/op
PASS
ok github.com/gofiber/fiber/v2 9.871s

[gaby]

@muratmirgun Can you take a look at the ci failures

[ReneWerner87]

move the functionality to the utils folder

if you can then please add the function for all parsers and add a config setting

[muratmirgun]

move the functionality to the utils folder

if you can then please add the function for all parsers and add a config setting

Okey I am working on it! 🚀

[ReneWerner87]

just change the PR type from draft in the normal one, when you are finished with the PR

[muratmirgun]

just change the PR type from draft in the normal one, when you are finished with the PR

Yes I know thanks for remember 💯

[muratmirgun]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

[ReneWerner87]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

no problem, the pull request should be almost ready i have done the last steps with the other maintainers

health always comes first of course, hope you are well again

[ReneWerner87]

[muratmirgun]

Hi, I am sorry for being offline lately. I have had some health problems and I have seen your articles and now I am looking at the example given. @ReneWerner87

no problem, the pull request should be almost ready i have done the last steps with the other maintainers

health always comes first of course, hope you are well again

thank you very much 🙏 we will move forward again in the next issues🚀

[gaby]

Can this be RWMutex ?

[efectn]

Can this be RWMutex ?

It can. Then we should not have to be waiting for writing lock

[ReneWerner87]

yes you are right

[ReneWerner87]

@muratmirgun maybe you can help, i found out 2 important things through the review hints

1. something is not thread safe -> added parallel test execution and analyzed the problems, unfortunately I can't find the right solution
   if i try to correct it i get into a deadlock when running the tests because something is wrong with the mutex hirachy

2. the defaults cannot be applied deeply, which is what you would expect if you use the function from the utils directly, for example
   -> I have created a test that proves this and helps to correct the problem

furthermore, i noticed that the performance could be optimized even further, instead of parsing and converting the value from the default tag every time, you could keep it or create a slice with closures (instead of the structElementCache), which is later only run through with the element and sets the values

therefore the go-defaults also used the filler

however, the biggest problem is that the functionality is not thread safe

[muratmirgun]

I saw your last commits and new test cases I am looking for your comment maybe we can make a mechanism where we can separate the default parts. I've started working on it. @ReneWerner87 . I will push new methods asap

[muratmirgun]

I am looking the last changes from this pr for sync

[ReneWerner87]

another good library
https://github.com/creasty/defaults/tree/master

#2571 (comment)
I'm asking myself again whether we should really integrate this, as the performance could suffer and you can simply use it from outside

import "github.com/creasty/defaults"

and

defaults.Set(myStruct)

is enough to do it outside

[muratmirgun]

that's a good question, I made a structure like this at first with the aim of less dependency because a performance problem on the library side can affect us very seriously. we must be in control. if I can't solve the thread safe problem, I will also examine the structures in this repo on the side. Because

[ReneWerner87]

we exclude this feature from the current release for now
if you find a good solution we can put it in the next version or v3

by the way, it is planned to be the last v2 release
but maybe we can still use it as an after feature

[ReneWerner87]

@muratmirgun can you work on the last notes? in the v3 branch or should we rather just add documentation and recommend other packages?

[efectn]

Hi @muratmirgun the PR is now closed as default feature was added in the last versions of Gorilla schema and it's been just updated for Fiber. Anyway, thanks for your contribution!