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

Add ability to limit the number of generated atoms #307

Closed

Conversation

Maria-12648430
Copy link

As mentioned in #290, the usage of the atom generator (explicit or implicit) can lead to exhaustion of the atom table. While I understand that PropEr does not want to restrict itself to just afew atoms per se, it would be very useful if a user (like me) was given the choice to limit this if needed.

src/proper_gen.erl Outdated Show resolved Hide resolved
@Maria-12648430 Maria-12648430 marked this pull request as ready for review March 22, 2023 07:58
@Maria-12648430 Maria-12648430 requested a review from kostis March 22, 2023 09:01
Copy link
Collaborator

@kostis kostis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure there is something to review at this point: this PR comes with no tests for specifying the atom_limit option, even with infinity, so I have zero confidence that the code does anything reasonable when that option is set.

When you do add some tests, you will discover that the code for the non-infinity case cannot run on older Erlang/OTP versions (e.g. 21 and 22 - I believe) because persistent_term is not available there. So, this code cannot be included as is as long as PropEr still supports these Erlang/OTP versions...

@Maria-12648430
Copy link
Author

Maria-12648430 commented Mar 22, 2023

When you do add some tests, you will discover that the code for the non-infinity case cannot run on older Erlang/OTP versions (e.g. 21 and 22 - I believe) because persistent_term is not available there. So, this code cannot be included as is as long as PropEr still supports these Erlang/OTP versions...

For that case, I suggest we put in a conditional that falls back to always using infinity (and ignore the setting) when used with an older OTP version that does not support persistent_term. Would that be acceptable?

EDIT: persistent_term was introduced in OTP 21.2.

@kostis
Copy link
Collaborator

kostis commented Mar 22, 2023

EDIT: persistent_term was introduced in OTP 21.2.

OK - so then it's not that bad as I feared.
In that case, yes, the conditional you suggest will do.

@Maria-12648430
Copy link
Author

I'll retract this PR for the time being. Having tried to make the existing tests work, I realized that it changes the behavior of the current atom generator to a considerable degree, especially when it comes to shrinking, which is currently tightly related to list and integer shrinking.

However, as stated in #290 and the follow-up discussion in the Erlang Forums, @juhlig's and my general opinion is that...

  • Generating atoms without restraint is, obviously, dangerous (and no, the non-garbage-collection behavior of atoms in Erlang itself won't change anytime soon).
  • Any atom is basically as good, or bad for that matter, as any other. There is no difference in complexity based on the length or the number that makes up any of its characters, between an empty atom and an atom composed of 255 255s.
  • There are the well-known and universally used atoms like undefined, ok, error, infinity, true, false and whatnot; they are the most likely to cause trouble, but it is very unlikely that one of them is created by an atom generator that produces just random strings and turns them into atoms. For example, the probability that the two-character atom ok is generated by a generator operating at size 2 is about 0.0015%.
  • Conversely, there are legions of atoms being generated where any of them is as (un-)likely to be problematic as any other.

For those reasons, it is our opinion that the current way of atom generation is, to put it bluntly, pretty useless, and that it needs to be reconsidered and redesigned. Which is no easy task, because in a realm wherein anything is as good or bad as anything else, what can you reduce to and shrink on?

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.

2 participants