p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
681 stars 445 forks source link

Remove IR::Annotations and make IAnnotated to carry annotations inline #4992

Closed asl closed 2 weeks ago

asl commented 3 weeks ago

Lots of cleanups and fixes here and there

asl commented 3 weeks ago

This is PR on top of https://github.com/p4lang/p4c/pull/4971 for simplicity.

As mentioned in https://github.com/p4lang/p4c/issues/4974, IR::Annotations incur a huge overhead, and in general does not make any sense: it's just another level of indirection. So, this just implements old FIXME, plus does some cleanup here and there.

The impact of # of allocations is huge. Before: image After:

Screenshot 2024-11-01 at 15 49 14

So, we're 10% less of memory allocations by both total size and number.

The runtime impact with GC disabled is huge as well: Command Mean [s] Min [s] Max [s] Relative
test/gtestp4c-nogc-main --gtest_filter=P4CParserUnroll.switch_20160512 3.240 ± 0.048 3.195 3.422 1.10 ± 0.03
test/gtestp4c-nogc --gtest_filter=P4CParserUnroll.switch_20160512 2.948 ± 0.061 2.855 3.041 1.00

So, we're 10% faster here (!)

Runtime impact with GC on is much less visible due to huge GC overhead (1.5x): Command Mean [s] Min [s] Max [s] Relative
test/gtestp4c-main --gtest_filter=P4CParserUnroll.switch_20160512 4.584 ± 0.099 4.312 4.897 1.02 ± 0.03
test/gtestp4c --gtest_filter=P4CParserUnroll.switch_20160512 4.512 ± 0.072 4.309 4.649 1.00
asl commented 3 weeks ago

sanitizers are broken after https://github.com/p4lang/p4c/pull/4989

asl commented 2 weeks ago

I rebased the branch after abseil string helpers were merged in. Now it should show the changes more cleanly

asl commented 2 weeks ago

validate-p4 fixes are in https://github.com/p4gauntlet/toz3/pull/41

fruffy commented 2 weeks ago

@smolkaj fyi. This might influence the way you work with annotations.

vlstill commented 1 week ago

@asl, apparently this breaks dbprint of annotated types, e.g. header of a function now can look like this in dbprint: { }void foo({ }in int<16> bar, { }in int<16> baz, { }out int<16> boo). Presumably the problem is that if annotations member is just printed, it will be printed using IR::Vector::dbprint, which is printed as bracket-encodes list with , separators.

asl commented 1 week ago

@vlstill The debug printing of annotations was pretty broken before this PR. In particular it only prints expr, so unparsed or structured annotations are not printed at all(!).

I'm working on the proper fix when porting Annotation to disjoint union. Stay tuned :)