refactor(router/atc): do not create new table in split_routes_and_services_by_path()

[chronolaw]

Summary

The original split_routes_and_services_by_path() will create a new table to contains all routes even if no splitting.
To erase this overhead, now split_routes_and_services_by_path() will insert the cloned routes directly into the passed in table.

KAG-4138

Checklist

• The Pull Request has tests
• A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

[chronolaw]

@hanshuebner , could you take a look? since you are the author of this feature.

[hanshuebner]

What is the benefit of this change. I find that the split_routes_and_services_by_path function was already pretty complicated, and the change further complicates it. I would endorse this change only if the performance benefit is measureable and significant.

[chronolaw]

I did a simple test:

  it("#only test", function()
    local transform = require("kong.router.transform")
    local split_routes_and_services_by_path = transform.split_routes_and_services_by_path

    local routes = {}

    for i = 1, 5000 do
      routes[i] = {
        route = {
          id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
          hosts = { "a.com" },
        }
      }
    end

    local COUNT = 1000

    ngx.update_time()
    local t = ngx.now()

    for i = 1, COUNT do
      split_routes_and_services_by_path(routes)
    end

    ngx.update_time()
    print(ngx.now() - t)
  end)

The master's output:

(kong-dev) [KONG kong]$bin/busted t.lua -t only
0.03000020980835
●
1 success / 0 failures / 0 errors / 0 pending : 0.063314 seconds
(kong-dev) [KONG kong]$bin/busted t.lua -t only
0.028000116348267
●
1 success / 0 failures / 0 errors / 0 pending : 0.059201 seconds

This PR's output:

(kong-dev) [KONG kong]$bin/busted t.lua -t only
0.017999887466431
●
1 success / 0 failures / 0 errors / 0 pending : 0.049651 seconds
(kong-dev) [KONG kong]$bin/busted t.lua -t only
0.016999959945679
●
1 success / 0 failures / 0 errors / 0 pending : 0.053319 seconds

Obviously, This PR is faster.
Of course, it is a corner case, there is no paths in the route, so This PR will not insert any data into the table.

@hanshuebner , Do you have more opinions?

[hanshuebner]

I think the performance optimization is too visible in the code, making it harder to understand and follow. It was already pretty difficult to understand. Given the tiny benefit, I would not go ahead and merge it.

[bungle]

@chronolaw, I guess you will get the idea?

[bungle]

@chronolaw here are some of my proposals, feel free to merge them here or ignore:
#12901

[chronolaw]

@chronolaw here are some of my proposals, feel free to merge them here or ignore: #12901

Thank you Aapo, it is a good improvement.
But I think it also increase the complexity of this PR, could you turn it into a new PR after this PR is merged?

[bungle]

@chronolaw here are some of my proposals, feel free to merge them here or ignore: #12901

Thank you Aapo, it is a good improvement. But I think it also increase the complexity of this PR, could you turn it into a new PR after this PR is merged?

Ok.

[team-gateway-bot]

Successfully created cherry-pick PR for master:

• https://github.com/kong/kong-ee/pull/8916