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

Remove profiling limit on files with <= 2^16 lines #1913

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

fingolfin
Copy link
Member

The new limit is 2^31, which should be sufficient for everybody (TM).

It exploits the fact that all statements in a T_BODY come from the same file, and that file is being recored in the T_BODY bag anyway.

Previously, the filename was stored as a string object in the T_BODY. This still is supported (and used by some code), but for regular GAP functions, we now instead store an immediate integer indexing into the filecache. This is so that profiling code does not have to be modified to heavily.

The new limit is 2^31, which should be sufficient for everybody (TM).
@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: kernel labels Nov 16, 2017
@fingolfin
Copy link
Member Author

@ChrisJefferson any thoughts on this?

@ChrisJefferson
Copy link
Contributor

My concern is that I'm not completely sure when we get a string, and when we get an int, and exactly how that effects various things. Profiling will never output anything about lines where we store a string instead of an ID. When does that happen, and should it worry us?

@fingolfin
Copy link
Member Author

It will happen if and only if it happened before. Which I think is never for actual GAP functions (and always for non-GAP functions, just like before). We could add a GAP_ASSERT to verify that.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Nov 21, 2017
@ChrisJefferson ChrisJefferson merged commit 1d0042b into gap-system:master Nov 21, 2017
@fingolfin fingolfin deleted the mh/gapnameid branch November 21, 2017 17:48
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants