You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Apr 2, 2022. It is now read-only.
One thing I was thinking about if we can make convert functions optional (ValueOption). My understanding that it is id function mist of the time. If that is true, skipping calling it with checking for ValueSome is probably worth it in terms of perf.
BUT! it is probably going to be tricky to type properly, given that then we will have to convert 'modelType -> 'valueType.
So maybe a union can be a solution there.
just a sketch:
typeScalarAttributeDefinition<'inputType,'modelType,'valueType>=| NoConverters of....// 'inputType = 'modelType = 'valueType, are all the same| JustModelConverted of...// 'inputType <> 'modelType , 'valueType ='modelType| JustValueConverter of...// 'inputType = 'modelType , 'valueType <> 'modelType| ModelAndValueConverters of...// 'inputType <> 'modelType <> 'valueType, are all different
The text was updated successfully, but these errors were encountered:
TimLariviere
changed the title
[Optimization] Avoid dynamic dispatch for Convert / ConvertValue in AttributeDefinition
[Performance] Avoid dynamic dispatch for Convert / ConvertValue in AttributeDefinition
Jan 14, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
One thing I was thinking about if we can make convert functions optional (
ValueOption
). My understanding that it isid
function mist of the time. If that is true, skipping calling it with checking forValueSome
is probably worth it in terms of perf.BUT! it is probably going to be tricky to type properly, given that then we will have to convert
'modelType -> 'valueType
.So maybe a union can be a solution there.
just a sketch:
Feels very clunky but possible worth considering
Originally posted by @twop in #33 (comment)
The text was updated successfully, but these errors were encountered: