Skip to content

Commit

Permalink
perf(schema) no deep copy on select on process auto fields
Browse files Browse the repository at this point in the history
### Summary

It is inefficient to create deep copies of tables when e.g. looping through
database rows. In my testing with uploading some 64k routes with dbless
(which calls process auto fields twice), this can cut the time looping the
data by 1/4th. It also generates much less garbage.

I searched our code bases where we use "select" context, and could not
find anything that might break because of this.
  • Loading branch information
bungle committed May 17, 2022
1 parent 88e60a2 commit 337122e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
10 changes: 6 additions & 4 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1609,11 +1609,15 @@ end
-- valid values are: "insert", "update", "upsert", "select"
-- @param nulls boolean: return nulls as explicit ngx.null values
-- @return A new table, with the auto fields containing
-- appropriate updated values.
-- appropriate updated values (except for "select" context
-- it does it in place by modifying the data directly).
function Schema:process_auto_fields(data, context, nulls, opts)
local check_immutable_fields = false

data = tablex.deepcopy(data)
local is_select = context == "select"
if not is_select then
data = tablex.deepcopy(data)
end

local shorthand_fields = self.shorthand_fields
if shorthand_fields then
Expand Down Expand Up @@ -1664,8 +1668,6 @@ function Schema:process_auto_fields(data, context, nulls, opts)
local now_s
local now_ms

local is_select = context == "select"

-- We don't want to resolve references on control planes
-- and and admin api requests, admin api request could be
-- detected with ngx.ctx.KONG_PHASE, but to limit context
Expand Down
35 changes: 35 additions & 0 deletions spec/01-unit/01-db/01-schema/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,41 @@ describe("schema", function()
end)

describe("process_auto_fields", function()
for _, context in ipairs({ "insert", "update", "upsert"}) do
it('returns new table when called with "' .. context .. '" context', function()
local Test = Schema.new({
fields = {
{ f = { type = "string", default = "test" } },
}
})

local original = {}
local data, err = Test:process_auto_fields(original, context)
assert.is_nil(err)
assert.not_equal(original, data)
if context == "update" then
assert.is_nil(data.f)
else
assert.equal("test", data.f)
end
assert.is_nil(original.f)
end)
end

it('modifies table in place when called with "select" context', function()
local Test = Schema.new({
fields = {
{ f = { type = "string", default = "test" } },
}
})

local original = {}
local data, err = Test:process_auto_fields(original, "select")
assert.is_nil(err)
assert.equal(original, data)
assert.equal("test", data.f)
assert.equal("test", original.f)
end)

it("produces ngx.null for non-required fields", function()
local Test = Schema.new({
Expand Down

0 comments on commit 337122e

Please sign in to comment.