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

auto-v_dont_optimize for vcl_init only functions does not quite work yet #3545

Closed
nigoroll opened this issue Mar 8, 2021 · 0 comments
Closed
Assignees

Comments

@nigoroll
Copy link
Member

nigoroll commented Mar 8, 2021

The intention behind 8551b1f was to ensure that we add v_dont_optimize for subs called from housekeeping subs, but only if they are called from there only (in other words, if a sub is called both from housekeeping and a any other sub, we do want the compiler to optimize).
This does work for a case like

vcl 4.1;
backend foo None;
import directors;

sub a {
	new d = directors.shard();
}

sub vcl_init {
	call a;
}

because the new keyword makes the sub "init-only":

void v_dont_optimize v_matchproto_(vcl_func_f)
VGC_function_a(VRT_CTX, enum vcl_func_call_e call,
    enum vcl_func_fail_e *failp)
{
  assert(call == VSUB_STATIC);
  assert(failp == NULL);
  assert(ctx->method & (VCL_MET_INIT));
...

however, if a sub does not have new and is still called from vcl_init only, it will not get the attribute:

vcl 4.1;
backend foo None;

sub a { }

sub vcl_init {
	call a;
}

->

void v_matchproto_(vcl_func_f)
VGC_function_a(VRT_CTX, enum vcl_func_call_e call,
    enum vcl_func_fail_e *failp)
{
  assert(call == VSUB_STATIC);
  assert(failp == NULL);
  assert(ctx->method & (VCL_MET_BACKEND_ERROR|VCL_MET_BACKEND_FETCH|VCL_MET_BACKEND_RESPONSE|VCL_MET_DELIVER|VCL_MET_FINI|VCL_MET_HASH|VCL_MET_HIT|VCL_MET_INIT|VCL_MET_MISS|VCL_MET_PASS|VCL_MET_PIPE|VCL_MET_PURGE|VCL_MET_RECV|VCL_MET_SYNTH));

The reason is that the okmask allows it to be called from anywhere. But v_dont_optimize should not be determined by from where can be called, but rather by from where it actually is called.

@nigoroll nigoroll self-assigned this Mar 8, 2021
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

No branches or pull requests

1 participant