Open BrunoMSantos opened 1 year ago
Very cool! One thing that I wondering: if we parse the same header file twice in sphinx (e.g. through by directly referencing two different symbols from the same file), does this caching also apply? I think probably not? That would be something that could considerably speed up the build in certain scenarios.
I believe it will reuse it as long as it's in the context of a single import
and as long as Clang produces the cursor hashes deterministically. I believe it's a yes on the 1st, and I strongly suspect it's a yes on the second one too, but I didn't test it out.
Though the bigger issue with caching in that case is the caching of the clang parsing stage, which is still not a fully global cache yet. You'll find some references to that if you look around in the issues / old PRs.
I believe it will reuse it as long as it's in the context of a single
import
and as long as Clang produces the cursor hashes deterministically. I believe it's a yes on the 1st, and I strongly suspect it's a yes on the second one too, but I didn't test it out.
Cool. I will test it out on my code and post some results here :)
Though the bigger issue with caching in that case is the caching of the clang parsing stage, which is still not a fully global cache yet. You'll find some references to that if you look around in the issues / old PRs.
Ah, you probably mean https://github.com/jnikula/hawkmoth/pull/75?
Hm, doesn't seem to be faster:
Without cache:
real 0m16,047s
user 0m15,493s
sys 0m0,483s
With cache:
real 0m16,607s
user 0m15,640s
sys 0m0,878s
I'm reading the 5 different files roughly 2 times on average.
Ah, you probably mean https://github.com/jnikula/hawkmoth/pull/75?
Yup, but there's also #157 and #171 at least ;)
Hm, doesn't seem to be faster:
I wouldn't expect otherwise. It takes a lot of compounding to make it noticeable, and your use case seems too small. Can you artificially create a test case in which you go over the same file like a 100 times? Otherwise you're getting the overhead and none of the benefits.
Ideally, the overhead won't matter in small cases, but the benefits will kick in in force for the big cases, which would make this viable. And there are of course other scenarios in which it might matter: we plan on exposing the cursor to the user through extensions for instance, allowing the user to exponentially grow the number of calls to these methods.
Here is the new cache with the
functools
decorator. As expected, that pitfall was actually the feature I wanted all along. 2 equalDocCursor
instances are able to reuse the same values, which is a win when iterating over children / parents when traversing the AST, possibly multiple times.From limited testing and looking at the code, I have the impression this currently benefits C++ code more than C (greater need to look into parents and children of any given cursor, constantly hitting the same types and so on). With instrumentation of the code, I saw some methods save 50+% of all calls when running the test suite, but I need to look at it better. This is meant only for show and tell at this point.
Sadly this did not improve performance (still over the test suite) in any measurable way. If anything it makes it ever so slightly worse (some <0.1s per run on average on my machine).
Some of the cached methods were never triggered twice for the same cursor within our code, so I tried enabling cache only for those that showed promising numbers. But no measurable difference there, so this branch is enabling cache for all methods that are called from more than one place in our code and / or are directly exposed through the cursor's API (with one exception as noted in the relevant commit).
Performance wise this doesn't worry me, I think it's worth it for future proofing all sorts of use cases within events and so on. But not sure all methods deserve an equally large maximum cache size or what the defaults should be. Also don't know hoe to best expose tuning and profiling values to the user.