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

Import profiler.enable documentation from xhgui project #20

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Jul 12, 2020

@glensc
Copy link
Contributor Author

glensc commented Jul 12, 2020

I'm thinking of having 'profiler.enable' also to accept boolean true/false values for simplicity. and having default value to be true.

@markstory wdyt?

@Krinkle
Copy link
Contributor

Krinkle commented Jul 12, 2020

Perhaps the non-function value could be an integer rather than boolean, e.g. for population size (enable=1 is on, enable=0 is off, enable=100 is 1:100 sampled); or a float for sample rate (e.g. enable=0.01 is 1/100).

Having said that, I'm also all for minimalism and letting the caller own and provide the sampling method. So perhaps that's unnecessary.

@glensc
Copy link
Contributor Author

glensc commented Jul 12, 2020

@Krinkle seems un-necessary to put the logic to library. people start to complain if it doesn't work right :) I would say that using rand is not pedantically accurate, to get exactly 10% of users you need to count them, which means the need for a shared database, and perhaps even locking :D

however, I do see value having simple "off" (0%) and "on" (100%) use.

@glensc glensc merged commit 4c9e312 into master Jul 13, 2020
@glensc glensc self-assigned this Jul 13, 2020
@glensc glensc deleted the profiling-rate branch July 13, 2020 15:07
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