Improve performance of interpolation in decalarative sources

[szubster]

What

interpolated_string.py is called thousands or even millions of times in declarative streams.
E.g. in datetime_based_cursor.py.
It is often the case (e.g. in jira source) that there is no evaluation needed, and input is just a string.
When encountered with multiple parent streams this can lead to jinja evaluation taking most of the CPU time.

Jinja compiled templates are also compiled on each evaluation. Which makes them slow.

How

Check whether evaluated string equals input on first try. If so, use input string directly.
Add cache for compiled jinja templates.

Review guide

1. interpolated_string.py

User Impact

Improved performance.
Example of small run on our jira instance (tests from changes in interpolated_string.py only:
pre:

Executed in  267.01 secs    fish           external
   usr time  153.66 secs    0.11 millis  153.66 secs
   sys time    1.10 secs    1.35 millis    1.10 secs

post:

Executed in  130.13 secs    fish           external
   usr time   24.47 secs    0.18 millis   24.47 secs
   sys time    0.79 secs    2.05 millis    0.78 secs

Difference in test execution for whole interpolated package (with jinja improvements):

	before	after
single eval	0.75s	0.76s
loop 100 evals on single object	14.93s	1.36s

Can this PR be safely reverted and rolled back?

• YES 💚
• NO ❌

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 21, 2024 8:06pm

[CLAassistant]

All committers have signed the CLA.

[natikgadzhi]

This is a huge gain 👏

[natikgadzhi]

Suggested change

	self._just_string = None
	self._is_plain_string = None

Naming doesn't sit right with me — something like this perhaps? Up to @artem1205 and @girarda though.

[szubster]

I like this one

[natikgadzhi]

Since this does a bit of caching magic, I'd love a comment explaining why you're doing this in eval specifically. I understand you need config to actually interpolate, so this is not insane, but a comment for posterity would be good.

Also, are there scenarios where the first interpolation would render the same string, but later interpolation would render something different? I can't think of it, but is there a world where a jinja expression renders itself / morphs conditionally on an input?

[girarda]

+1. The change makes sense to me, but let's describe the rationale for posterity.

That being said, thank you for taking the time to contribute to Airbyte @szubster ! <3

[szubster]

If there is any (which I doubt) then the output would still not look like input?
There are code-challenges, like https://en.wikipedia.org/wiki/Quine_(computing) but I doubt it's a real-life scenario.
And even if it is, then the logic is still correct :)

[szubster]

Thank you for the comments.
I think I have found few more tricks to speed whole jinja interpolation. Not just the simple case.
Give me some time and I will come back to this.

[szubster]

Is there some synthetic benchmark or a test suite I could use to test those changes?

[girarda]

@szubster there aren't performance benchmark tests, but depending on where your follow up changes land:

• there's some tests specific to InterpolatedString
• there's some tests for the underlying interpolation logic

We'll also run regression tests with a few connectors to make sure this doesn't introduce regressions

[szubster]

All checks are passing now :)

[girarda]

Running regression tests for a few connectors. I'll merge after they run successfully 🎉

• https://github.com/airbytehq/airbyte/actions/runs/10496661776
• https://github.com/airbytehq/airbyte/actions/runs/10496674783
• https://github.com/airbytehq/airbyte/actions/runs/10496688648

[szubster]

There is one failure, but I do not see how it could be related to my change?
Error: Invalid value for '--name': 'source-quickbook' in ....

[natikgadzhi]

It should be quickbooks — must be a typo ;)

[girarda]

@szubster this is now available in CDK 4.5.3 🎉

Thank you for the huge improvement <3

[szubster]

If you would have any numbers from synthetic tests or anything to share how it improved performance, I would be glad to hear :)