Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(router): remove NYI to be more JIT-friendly #12467

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

ADD-SP
Copy link
Contributor

@ADD-SP ADD-SP commented Jan 31, 2024

Please SQUASH AND MERGE

Summary

---- TRACE 1772 start fields.lua:457
0001  UGET     4   0      ; FIELDS_FUNCS       (fields.lua:458)
0002  TGETV    4   4   1       (fields.lua:458)
0003  IST          4       (fields.lua:458)
0004  JMP      5 => 0009
0005  UGET     4   1      ; get_field_accessor       (fields.lua:459)
0006  TGETS    6   0   0  ; "funcs"       (fields.lua:459)
0007  MOV      7   1       (fields.lua:459)
0008  CALL     4   2   3       (fields.lua:459)
0000  . FUNCF    9          ; fields.lua:266
0001  . TGETV    2   0   1       (fields.lua:267)
0002  . ISF          2       (fields.lua:268)
0003  . JMP      3 => 0005
0004  . UCLO     0 => 0055       (fields.lua:269)
---- TRACE 1772 abort fields.lua:269 -- NYI: bytecode 50

Generating a field accessor in place in the get_field_accessor() is not JIT friendly, so let' generate accessor from the other function.


And tail call is not JIT-friendly in some cases, so rewrote it.

Checklist

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

Issue reference

KAG-3653

@ADD-SP ADD-SP added cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee backport release/3.6.x labels Jan 31, 2024
@ADD-SP ADD-SP requested a review from chronolaw January 31, 2024 08:28
@ADD-SP ADD-SP changed the title perf(router): remove NYI to be more JIT friendly perf(router): remove NYI to be more JIT-friendly Jan 31, 2024
@ADD-SP ADD-SP force-pushed the add_sp/perf-get-rid-off-nyi branch from 6353fdd to 14fc1fe Compare January 31, 2024 08:34
@ADD-SP ADD-SP force-pushed the add_sp/perf-get-rid-off-nyi branch from 14fc1fe to 9e4947e Compare January 31, 2024 09:03
@chronolaw chronolaw marked this pull request as ready for review January 31, 2024 09:17
@bungle
Copy link
Member

bungle commented Jan 31, 2024

Not sure if CI is flaky, but seems like this may have caused issue. Restarting CI 3rd time.

@bungle
Copy link
Member

bungle commented Jan 31, 2024

Not sure if CI is flaky, but seems like this may have caused issue. Restarting CI 3rd time.

Seems to gone green now.

@bungle
Copy link
Member

bungle commented Jan 31, 2024

Today is code freeze, so I am not sure do we want this in still? Seems like a stretch. How much this improves anything?

@ADD-SP ADD-SP marked this pull request as draft February 1, 2024 02:03
@chronolaw chronolaw force-pushed the add_sp/perf-get-rid-off-nyi branch from d5e6b1f to edd163f Compare March 6, 2024 07:16
@chronolaw chronolaw marked this pull request as ready for review March 6, 2024 07:19
@bungle
Copy link
Member

bungle commented Mar 13, 2024

@ADD-SP are we going to merge this? Ping @chronolaw, can you review and approve?

@chronolaw
Copy link
Contributor

@ADD-SP are we going to merge this? Ping @chronolaw, can you review and approve?

I already approved this PR and did some work on it, I think that we can merge it.

@ADD-SP ADD-SP force-pushed the add_sp/perf-get-rid-off-nyi branch from 192506e to 66b56cd Compare March 14, 2024 01:54
@ADD-SP
Copy link
Contributor Author

ADD-SP commented Mar 14, 2024

Merge on CI green

@ADD-SP ADD-SP merged commit c72f980 into master Mar 14, 2024
26 checks passed
@ADD-SP ADD-SP deleted the add_sp/perf-get-rid-off-nyi branch March 14, 2024 03:29
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/performance core/router size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants