fix resource level max_table_nesting and normalizer performance tuning

[sh-rp]

Description

The resource level max_table_nesting settings where not passed down to child tables, this PR fixes this. I also completely rewrote the tests to be much more readable (imho), also they were not testing various cases although they were claiming to.

[netlify]

✅ Deploy Preview for dlt-hub-docs canceled.

Name	Link
🔨 Latest commit	8246012
🔍 Latest deploy log	https://app.netlify.com/sites/dlt-hub-docs/deploys/672b5f8ce20de6000810b67f

[sh-rp]

should we check intermediary tables here too? so if parent path is ["one", "two"] should we also check table "one__two" or is it always correct to use the top table? I'm not sure there is a case where there would be settings in a intermediary table.

[rudolfix]

we need only parent table. the overall fix is good but maybe we could optimize this block a little? it is always top level table that has this setting so maybe we should pass root table as table_name here? then no changes are needed

[rudolfix]

this is good just look at performance again because function here are often called

[rudolfix]

we need only parent table. the overall fix is good but maybe we could optimize this block a little? it is always top level table that has this setting so maybe we should pass root table as table_name here? then no changes are needed

[rudolfix]

this is probably the most often called function in our code. maybe we can skip passing additional argument here or do something else to make it faster?

• count the recursion level down (r_lvl) with the initial value initialized to max nesting per table. so we do not unnest when below _r_lvl
• pass just parent table name, should be faster than tuple

[sh-rp]

@rudolfix I think I have taken all your ideas into account and it should be fast now, I need to run some benchmarks though.

[sh-rp]

For the following code the time goes from 4.5s on devel to about 3.1 seconds on this branch with the normalization caching. The boost mainly comes from cached normalization, I think only to a very small degree from the nesting stuff. String operations just are expensive.

import json
import os
import time
from tests.common.utils import json_case_path
from dlt.common.schema import Schema

def rasa_event_bot_metadata():
    with open(json_case_path("rasa_event_bot_metadata"), "rb") as f:
        return json.load(f)
    
def norm():
    return Schema("default").data_item_normalizer  # type: ignore[return-value]

if __name__ == "__main__":
    
    payload = rasa_event_bot_metadata()

    for i in range(1):
        n = norm() 
        start = time.time()
        for i in range(1000):
            payload["_id"] = i
            list(n.normalize_data_item(payload, "load_id", "table"))
        print(f"Time taken: {time.time() - start}")

[sh-rp]

this adds a considerable speed boost. we could also consider adding caching support on the naming and not here so that all normalizers and other places can benefit, I'm not quite sure if there are other places where this gets called as often as here though.

[rudolfix]

this is interesting because this functions is already cached. are you using snake_case? convention? please look at the underlying code again

[rudolfix]

please see my comments. let's check where the improvements are coming from, becasue I already cache ident normalizers...

[rudolfix]

this is interesting because this functions is already cached. are you using snake_case? convention? please look at the underlying code again

[rudolfix]

this is also cached already

[sh-rp]

@rudolfix it's using snake_case naming. There are a few of operations that get called over and over again because they are not cached. For example if you call normalize_identifier, then this is not cached, only _normalize_identifier is. The snake_case class calls the super class where strip is called and some checks are made. The strip alone accounts for 0.2s of those 1.7s or so gains. Then for example in shorten_fragments make_paths is calculated over and over again which also accounts for a bit of the savings etc. I can try to move all the caching to the naming if you like.

[rudolfix]

OK! there are other naming conventions that are not cached so that will speed things up. All non-deterministic naming conventions will stop working but no one should write those :)