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

Improve generation of unique strings #3350

Merged
merged 2 commits into from
Aug 8, 2022
Merged

Improve generation of unique strings #3350

merged 2 commits into from
Aug 8, 2022

Conversation

davidbolvansky
Copy link
Contributor

No description provided.

lib/cstring.h Outdated Show resolved Hide resolved
@davidbolvansky davidbolvansky marked this pull request as ready for review June 13, 2022 15:36
@mihaibudiu
Copy link
Contributor

This change will cause hundreds of files generated by the compiler to change, cause the corresponding tests to change.
You need to submit these files as well.

@davidbolvansky
Copy link
Contributor Author

This change will cause hundreds of files generated by the compiler to change, cause the corresponding tests to change. You need to submit these files as well.

Is there any way how to autoregenerate outputs for failed tests?

@vhavel
Copy link

vhavel commented Aug 1, 2022

@davidbolvansky re-run failed tests (or whole test-suite) with P4TEST_REPLACE=True

We have just seen real P4 program which takes hours to compile. The source program is large, and is additionally inflated by some midend transformations used in our backend. The typeChecking on huge IRs then becomes realy slow.

If we don't want to change all the identifiers, we could also implement the unique string generation in typeChecking manually. Maintaining inUse set and counter in different functions seems to prevent the unique string generation to be implemented efficiently and cleanly.

@davidbolvansky
Copy link
Contributor Author

davidbolvansky commented Aug 1, 2022

Ok, to reduce test churn, I implemented other approach based on the usage of references to counters, as suggested by Vojta.

For every name, we track how many times it was used as a "base" for newly generated strings.

@mihaibudiu
Copy link
Contributor

Indeed, the compiler has not really been designed to be fast. I am not sure how much we can fix this now.
If this is ready for review please ask for one.
You need to document the new map structure, though.

@davidbolvansky
Copy link
Contributor Author

Yeah, I will document it soon.

@davidbolvansky
Copy link
Contributor Author

Indeed, the compiler has not really been designed to be fast. I am not sure how much we can fix this now.

Understood, but there are low hanging fruits (like this one) which can improve compile times a lot. We have a program where better unique string generation improves compile time significatly (2-3 hours -> 5 min).

@mihaibudiu mihaibudiu merged commit 2d63188 into p4lang:main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants