Hash enabled check to further improve performance (#353)

ory · Jan 27, 2020 · 19099cb · 19099cb
1 parent 6afdeae
commit 19099cb
Show file tree

Hide file tree

Showing 15 changed files with 81 additions and 7 deletions.
diff --git a/driver/configuration/provider_viper.go b/driver/configuration/provider_viper.go
@@ -104,15 +104,20 @@ const (
 )
 
 type ViperProvider struct {
-	l            logrus.FieldLogger
-	configMutex  sync.RWMutex
-	configCachge map[uint64]json.RawMessage
+	l logrus.FieldLogger
+
+	enabledMutex sync.RWMutex
+	enabledCache map[uint64]bool
+
+	configMutex sync.RWMutex
+	configCache map[uint64]json.RawMessage
 }
 
 func NewViperProvider(l logrus.FieldLogger) *ViperProvider {
 	return &ViperProvider{
 		l:            l,
-		configCachge: make(map[uint64]json.RawMessage),
+		enabledCache: make(map[uint64]bool),
+		configCache:  make(map[uint64]json.RawMessage),
 	}
 }
 
@@ -202,7 +207,24 @@ func (v *ViperProvider) ToScopeStrategy(value string, key string) fosite.ScopeSt
 }
 
 func (v *ViperProvider) pipelineIsEnabled(prefix, id string) bool {
-	return viperx.GetBool(v.l, fmt.Sprintf("%s.%s.enabled", prefix, id), false)
+	hash, err := v.hashPipelineConfig(prefix, id, nil)
+	if err != nil {
+		return false
+	}
+
+	v.enabledMutex.RLock()
+	e, ok := v.enabledCache[hash]
+	v.enabledMutex.RUnlock()
+
+	if ok {
+		return e
+	}
+
+	v.enabledMutex.Lock()
+	v.enabledCache[hash] = viperx.GetBool(v.l, fmt.Sprintf("%s.%s.enabled", prefix, id), false)
+	v.enabledMutex.Unlock()
+
+	return v.enabledCache[hash]
 }
 
 func (v *ViperProvider) hashPipelineConfig(prefix, id string, override json.RawMessage) (uint64, error) {
@@ -232,7 +254,7 @@ func (v *ViperProvider) PipelineConfig(prefix, id string, override json.RawMessa
 	}
 
 	v.configMutex.RLock()
-	c, ok := v.configCachge[hash]
+	c, ok := v.configCache[hash]
 	v.configMutex.RUnlock()
 
 	if ok {
@@ -300,7 +322,7 @@ func (v *ViperProvider) PipelineConfig(prefix, id string, override json.RawMessa
 	}
 
 	v.configMutex.Lock()
-	v.configCachge[hash] = marshalled
+	v.configCache[hash] = marshalled
 	v.configMutex.Unlock()
 
 	return nil

diff --git a/driver/configuration/provider_viper_public_test.go b/driver/configuration/provider_viper_public_test.go
@@ -123,6 +123,39 @@ func BenchmarkPipelineConfig(b *testing.B) {
 	}
 }
 
+/*
+go test ./... -v -bench=. -run BenchmarkPipelineEnabled -benchtime=10s
+
+v0.35.4
+11708	   1009975 ns/op
+
+v0.35.5
+18848694	       603 ns/op
+*/
+
+func BenchmarkPipelineEnabled(b *testing.B) {
+	viper.Reset()
+	viperx.InitializeConfig(
+		"oathkeeper",
+		"./../../docs/",
+		logrus.New(),
+	)
+
+	err := viperx.Validate(gojsonschema.NewReferenceLoader("file://../../.schemas/config.schema.json"))
+	if err != nil {
+		viperx.LoggerWithValidationErrorFields(logrus.New(), err).Error("unable to validate")
+	}
+	require.NoError(b, err)
+
+	p := NewViperProvider(logrus.New())
+
+	for n := 0; n < b.N; n++ {
+		p.AuthorizerIsEnabled("allow")
+		p.AuthenticatorIsEnabled("noop")
+		p.MutatorIsEnabled("noop")
+	}
+}
+
 func TestViperProvider(t *testing.T) {
 	viper.Reset()
 	viperx.InitializeConfig(

diff --git a/pipeline/authn/authenticator_anonymous_test.go b/pipeline/authn/authenticator_anonymous_test.go
@@ -58,6 +58,7 @@ func TestAuthenticatorAnonymous(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthenticatorAnonymousIsEnabled, true)
 		require.NoError(t, a.Validate(json.RawMessage(`{"subject":"foo"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorAnonymousIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"subject":"foo"}`)))
 	})

diff --git a/pipeline/authn/authenticator_noop_test.go b/pipeline/authn/authenticator_noop_test.go
@@ -49,6 +49,7 @@ func TestAuthenticatorNoop(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthenticatorNoopIsEnabled, true)
 		require.NoError(t, a.Validate(nil))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorNoopIsEnabled, false)
 		require.Error(t, a.Validate(nil))
 	})

diff --git a/pipeline/authn/authenticator_oauth2_client_credentials_test.go b/pipeline/authn/authenticator_oauth2_client_credentials_test.go
@@ -111,12 +111,15 @@ func TestAuthenticatorOAuth2ClientCredentials(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2ClientCredentialsIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"token_url":""}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2ClientCredentialsIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"token_url":"`+ts.URL+"/oauth2/token"+`"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2ClientCredentialsIsEnabled, true)
 		require.Error(t, a.Validate(json.RawMessage(`{"token_url":""}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2ClientCredentialsIsEnabled, true)
 		require.NoError(t, a.Validate(json.RawMessage(`{"token_url":"`+ts.URL+"/oauth2/token"+`"}`)))
 	})

diff --git a/pipeline/authn/authenticator_oauth2_introspection_test.go b/pipeline/authn/authenticator_oauth2_introspection_test.go
@@ -353,12 +353,15 @@ func TestAuthenticatorOAuth2Introspection(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2TokenIntrospectionIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"introspection_url":""}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2TokenIntrospectionIsEnabled, true)
 		require.Error(t, a.Validate(json.RawMessage(`{"introspection_url":""}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2TokenIntrospectionIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"introspection_url":"/oauth2/token"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorOAuth2TokenIntrospectionIsEnabled, true)
 		require.Error(t, a.Validate(json.RawMessage(`{"introspection_url":"/oauth2/token"}`)))
 	})

diff --git a/pipeline/authn/authenticator_unauthorized_test.go b/pipeline/authn/authenticator_unauthorized_test.go
@@ -50,6 +50,7 @@ func TestAuthenticatorBroken(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthenticatorUnauthorizedIsEnabled, true)
 		require.NoError(t, a.Validate(nil))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthenticatorUnauthorizedIsEnabled, false)
 		require.Error(t, a.Validate(nil))
 	})

diff --git a/pipeline/authz/authorizer_allow_test.go b/pipeline/authz/authorizer_allow_test.go
@@ -48,6 +48,7 @@ func TestAuthorizerAllow(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthorizerAllowIsEnabled, true)
 		require.NoError(t, a.Validate(nil))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthorizerAllowIsEnabled, false)
 		require.Error(t, a.Validate(nil))
 	})

diff --git a/pipeline/authz/authorizer_deny_test.go b/pipeline/authz/authorizer_deny_test.go
@@ -48,6 +48,7 @@ func TestAuthorizerDeny(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthorizerDenyIsEnabled, true)
 		require.NoError(t, a.Validate(nil))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthorizerDenyIsEnabled, false)
 		require.Error(t, a.Validate(nil))
 	})

diff --git a/pipeline/authz/keto_engine_acp_ory_test.go b/pipeline/authz/keto_engine_acp_ory_test.go
@@ -224,12 +224,15 @@ func TestAuthorizerKetoWarden(t *testing.T) {
 		viper.Set(configuration.ViperKeyAuthorizerKetoEngineACPORYIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"base_url":"","required_action":"foo","required_resource":"bar"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthorizerKetoEngineACPORYIsEnabled, true)
 		require.Error(t, a.Validate(json.RawMessage(`{"base_url":"","required_action":"foo","required_resource":"bar"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthorizerKetoEngineACPORYIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"base_url":"http://foo/bar","required_action":"foo","required_resource":"bar"}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyAuthorizerKetoEngineACPORYIsEnabled, true)
 		require.NoError(t, a.Validate(json.RawMessage(`{"base_url":"http://foo/bar","required_action":"foo","required_resource":"bar"}`)))
 	})

diff --git a/pipeline/mutate/mutator_cookie_test.go b/pipeline/mutate/mutator_cookie_test.go
@@ -209,6 +209,7 @@ func TestCredentialsIssuerCookies(t *testing.T) {
 			viper.Set(configuration.ViperKeyMutatorCookieIsEnabled, true)
 			require.Error(t, a.Validate(json.RawMessage(`{}`)))
 
+			viper.Reset()
 			viper.Set(configuration.ViperKeyMutatorCookieIsEnabled, false)
 			require.Error(t, a.Validate(json.RawMessage(`{"cookies":{}}`)))
 		})

diff --git a/pipeline/mutate/mutator_header_test.go b/pipeline/mutate/mutator_header_test.go
@@ -204,6 +204,7 @@ func TestCredentialsIssuerHeaders(t *testing.T) {
 		viper.Set(configuration.ViperKeyMutatorHeaderIsEnabled, true)
 		require.NoError(t, a.Validate(json.RawMessage(`{"headers":{}}`)))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyMutatorHeaderIsEnabled, false)
 		require.Error(t, a.Validate(json.RawMessage(`{"headers":{}}`)))
 	})

diff --git a/pipeline/mutate/mutator_hydrator_test.go b/pipeline/mutate/mutator_hydrator_test.go
@@ -351,6 +351,7 @@ func TestMutatorHydrator(t *testing.T) {
 			{enabled: true, shouldPass: true, apiUrl: "http://api/bar"},
 		} {
 			t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+				viper.Reset()
 				viper.Set(configuration.ViperKeyMutatorHydratorIsEnabled, testCase.enabled)
 
 				err := a.Validate(json.RawMessage(`{"api":{"url":"` + testCase.apiUrl + `"}}`))

diff --git a/pipeline/mutate/mutator_id_token_test.go b/pipeline/mutate/mutator_id_token_test.go
@@ -258,6 +258,7 @@ func TestMutatorIDToken(t *testing.T) {
 			{e: true, i: "http://baz/foo", j: "http://baz/foo", pass: true},
 		} {
 			t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
+				viper.Reset()
 				viper.Set(configuration.ViperKeyMutatorIDTokenIsEnabled, tc.e)
 				// viper.Set(configuration.ViperKeyMutatorIDTokenIssuerURL, tc.i)
 				// viper.Set(configuration.ViperKeyMutatorIDTokenJWKSURL, tc.j)

diff --git a/pipeline/mutate/mutator_noop_test.go b/pipeline/mutate/mutator_noop_test.go
@@ -55,6 +55,7 @@ func TestMutatorNoop(t *testing.T) {
 		viper.Set(configuration.ViperKeyMutatorNoopIsEnabled, true)
 		require.NoError(t, a.Validate(nil))
 
+		viper.Reset()
 		viper.Set(configuration.ViperKeyMutatorNoopIsEnabled, false)
 		require.Error(t, a.Validate(nil))
 	})