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

Performance improvements #639

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
615120c
Upgrade elm-review
jfmengels Jun 27, 2024
e06301e
Add elm-review-simplify
jfmengels Jun 27, 2024
8f8d78e
Apply simplifications from Simplify
jfmengels Jun 27, 2024
a64ba20
Avoid unnecessary List.map
jfmengels Jun 27, 2024
c4eb619
Count composite in one list pass
jfmengels Jun 27, 2024
a992324
Extract function to top-level
jfmengels Jun 27, 2024
45c9001
Stop counting siblings when we've seen 2
jfmengels Jun 27, 2024
a43da2b
Make hashedAliasName lazy
jfmengels Jun 27, 2024
f7972e9
Use string concatenation
jfmengels Jun 27, 2024
51065c3
Remove unnecessary Maybe
jfmengels Jun 27, 2024
b057631
Use let expressions rather than lambdas and piping
jfmengels Jun 27, 2024
f988768
Remove unnecessary parentheses
jfmengels Jun 27, 2024
e10aaba
Compute name once
jfmengels Jun 27, 2024
f8dc4bd
Combine pipes
jfmengels Jun 27, 2024
517e609
Combine pipes
jfmengels Jun 27, 2024
04dfbbe
Combine filterMap and fold
jfmengels Jun 27, 2024
1d529cd
Use simple calls
jfmengels Jun 27, 2024
e7c7b5f
Avoid intermediate list
jfmengels Jun 27, 2024
ef9ad6b
Merge filter and foldl
jfmengels Jun 27, 2024
e92e903
Combine maps
jfmengels Jun 27, 2024
b70d6e0
Move faster check to be checked first
jfmengels Jun 27, 2024
ba4bdd7
Extract to top-level helper
jfmengels Jun 27, 2024
21b75fc
Split condition into two
jfmengels Jun 27, 2024
2da03b5
Move let declarations closer to where they're used
jfmengels Jun 27, 2024
c2cd276
Clarify what forceMethod has to be
jfmengels Jun 27, 2024
87742d1
Remove unused argument
jfmengels Jun 27, 2024
faa13b1
Avoid curried functions when we can explicit all arguments
jfmengels Jun 27, 2024
000ceab
Avoid computing exhaustiveFailureMessage unnecessarily
jfmengels Jun 27, 2024
2c348e5
Find decoder when iterating through the list instead of creating a Dict
jfmengels Jun 27, 2024
ad05d1c
Remove usage of interpolation
jfmengels Jun 27, 2024
015b923
Remove usage of interpolation
jfmengels Jun 27, 2024
62e30ee
Avoid record update
jfmengels Jun 27, 2024
37aad13
Use let expressions rather than lambdas and piping
jfmengels Jun 27, 2024
4091b8f
Create emptyList constant
jfmengels Jun 27, 2024
70797d0
Use List.reverse rather than map+List.reverse
jfmengels Jun 27, 2024
ab5e2af
Create emptyDict constant
jfmengels Jun 27, 2024
9155e12
Combine Decode.map and Decode.andThen
jfmengels Jun 27, 2024
c72f3eb
Rename argument to be consistent
jfmengels Jun 27, 2024
ec63261
Enable NoPrematureLetComputation rule
jfmengels Jun 27, 2024
3404484
Use List.isEmpty
jfmengels Jun 27, 2024
e196f55
Move expression to where it would be used
jfmengels Jun 27, 2024
425a353
Move let declaration closer to where it's used
jfmengels Jun 27, 2024
c5e4bff
Destructure in arguments
jfmengels Jun 27, 2024
a6aa3d7
Move expression to where it would be used
jfmengels Jun 27, 2024
843e5dd
Allow combineMaybeList to stop early when encountering Nothing
jfmengels Jun 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions generator/review/elm.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,32 @@
"elm/core": "1.0.5",
"elm/json": "1.1.3",
"elm/project-metadata-utils": "1.0.2",
"jfmengels/elm-review": "2.9.0",
"jfmengels/elm-review-common": "1.3.0",
"jfmengels/elm-review-debug": "1.0.6",
"jfmengels/elm-review-unused": "1.1.26",
"sparksp/elm-review-imports": "1.0.1",
"stil4m/elm-syntax": "7.2.9"
"jfmengels/elm-review": "2.14.0",
"jfmengels/elm-review-common": "1.3.3",
"jfmengels/elm-review-debug": "1.0.8",
"jfmengels/elm-review-simplify": "2.1.4",
"jfmengels/elm-review-unused": "1.2.3",
"sparksp/elm-review-imports": "1.0.2",
"stil4m/elm-syntax": "7.3.2"
},
"indirect": {
"elm/bytes": "1.0.8",
"elm/html": "1.0.0",
"elm/parser": "1.1.0",
"elm/random": "1.0.0",
"elm/regex": "1.0.0",
"elm/time": "1.0.0",
"elm/virtual-dom": "1.0.3",
"elm-community/list-extra": "8.6.0",
"elm-explorations/test": "1.2.2",
"miniBill/elm-unicode": "1.0.2",
"elm-explorations/test": "2.2.0",
"miniBill/elm-unicode": "1.1.1",
"pzp1997/assoc-list": "1.0.0",
"rtfeldman/elm-hex": "1.0.0",
"stil4m/structured-writer": "1.0.3"
}
},
"test-dependencies": {
"direct": {
"elm-explorations/test": "1.2.2"
"elm-explorations/test": "2.2.0"
},
"indirect": {}
}
Expand Down
4 changes: 4 additions & 0 deletions generator/review/src/ReviewConfig.elm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import NoImportingEverything
import NoInconsistentAliases
import NoMissingTypeAnnotation
import NoModuleOnExposedNames
import NoPrematureLetComputation
import NoUnused.CustomTypeConstructorArgs
import NoUnused.CustomTypeConstructors
import NoUnused.Dependencies
Expand All @@ -26,6 +27,7 @@ import NoUnused.Modules
import NoUnused.Parameters
import NoUnused.Patterns
import NoUnused.Variables
import Simplify
import Review.Rule exposing (Rule)


Expand Down Expand Up @@ -56,4 +58,6 @@ config =
-- |> NoInconsistentAliases.noMissingAliases
-- |> NoInconsistentAliases.rule
, NoModuleOnExposedNames.rule
, Simplify.rule Simplify.defaults
, NoPrematureLetComputation.rule
]
6 changes: 2 additions & 4 deletions generator/src/Graphql/Generator/Decoder.elm
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,13 @@ generateDecoder context (Type.TypeReference referrableType isNullable) =
let
constructor =
context.apiSubmodule
++ [ "Scalar" ]
++ [ ClassCaseName.normalized customScalarName ]
++ [ "Scalar", ClassCaseName.normalized customScalarName ]
|> String.join "."
in
[ (context.scalarCodecsModule |> Maybe.withDefault (ModuleName.fromList (context.apiSubmodule ++ [ "ScalarCodecs" ])))
|> ModuleName.append "codecs"
, context.apiSubmodule
++ [ "Scalar" ]
++ [ "unwrapCodecs" ]
++ [ "Scalar", "unwrapCodecs" ]
|> String.join "."
, ".codec"
++ ClassCaseName.normalized customScalarName
Expand Down
4 changes: 2 additions & 2 deletions generator/src/Graphql/Generator/Group.elm
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ interfaceImplementorsDict : List TypeDefinition -> Dict String (List TypeDefinit
interfaceImplementorsDict typeDefs =
let
( interfaceTypes, objectTypes ) =
Tuple.pair typeDefs typeDefs
( typeDefs, typeDefs )
|> Tuple.mapBoth (List.filter Type.isInterfaceType) (List.filter Type.isObjectType)

interfaceImplementations : String -> List TypeDefinition -> List TypeDefinition
Expand All @@ -56,7 +56,7 @@ interfaceImplementorsDict typeDefs =
(\objectOrInterface ->
Type.interfacesImplemented objectOrInterface
|> List.map ClassCaseName.raw
|> List.any ((==) interfaceName)
|> List.member interfaceName
)
objectsAndInterfaces

Expand Down
8 changes: 3 additions & 5 deletions generator/src/Graphql/Generator/InputObjectFile.elm
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ generateFileContents context typeDefinitions =
typesToGenerate =
typeDefinitions
|> List.filterMap (isInputObject typeDefinitions)

fields =
typesToGenerate
|> List.concatMap .fields
in
if typesToGenerate == [] then
interpolate
Expand All @@ -59,7 +55,9 @@ placeholder =
{2}
"""
[ moduleName context |> String.join "."
, generateImports context fields
, typesToGenerate
|> List.concatMap .fields
|> generateImports context
, typesToGenerate
|> List.map (generateEncoderAndAlias context)
|> String.join "\n\n\n"
Expand Down
56 changes: 26 additions & 30 deletions generator/src/Graphql/Generator/InputObjectLoops.elm
Original file line number Diff line number Diff line change
Expand Up @@ -28,34 +28,32 @@ fieldIsCircular typeDefs inputObjectName fieldTypeRef =


fieldIsCircular_ : List ClassCaseName -> List TypeDefinition -> ClassCaseName -> TypeReference -> Bool
fieldIsCircular_ visitedNames typeDefs inputObjectName fieldTypeRef =
let
alreadyVisitedThis =
visitedNames
|> List.map ClassCaseName.raw
|> List.Extra.allDifferent
in
case fieldTypeRef of
TypeReference referrableType isNullable ->
case referrableType of
InputObjectRef inputObjectRefName ->
case lookupInputObject typeDefs inputObjectRefName of
Just ( name, fields ) ->
not alreadyVisitedThis
|| isRecursive inputObjectName fields
|| List.any
(fieldIsCircular_ (inputObjectName :: visitedNames) typeDefs name)
(fields |> List.map .typeRef)

Nothing ->
False

Type.List listTypeRef ->
fieldIsCircular_ visitedNames typeDefs inputObjectName listTypeRef

_ ->
fieldIsCircular_ visitedNames typeDefs inputObjectName (TypeReference referrableType isNullable) =
case referrableType of
InputObjectRef inputObjectRefName ->
case lookupInputObject typeDefs inputObjectRefName of
Just ( name, fields ) ->
let
alreadyVisitedThis =
visitedNames
|> List.map ClassCaseName.raw
|> List.Extra.allDifferent
in
not alreadyVisitedThis
|| isRecursive inputObjectName fields
|| List.any
(fieldIsCircular_ (inputObjectName :: visitedNames) typeDefs name)
(fields |> List.map .typeRef)

Nothing ->
False

Type.List listTypeRef ->
fieldIsCircular_ visitedNames typeDefs inputObjectName listTypeRef

_ ->
False


lookupInputObject : List TypeDefinition -> ClassCaseName -> Maybe ( ClassCaseName, List Field )
lookupInputObject typeDefs inputObjectName =
Expand Down Expand Up @@ -94,10 +92,8 @@ hasRecursiveRef inputObjectName referrableType =
InputObjectRef referredInputObjectName ->
inputObjectName == referredInputObjectName

Type.List listTypeRef ->
case listTypeRef of
Type.TypeReference listType isNullable ->
hasRecursiveRef inputObjectName listType
Type.List (Type.TypeReference listType isNullable) ->
hasRecursiveRef inputObjectName listType

Type.Scalar _ ->
False
Expand Down
12 changes: 5 additions & 7 deletions generator/src/Graphql/Generator/Let.elm
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,8 @@ generate letBindings =
""" {0} =
{1}"""
[ name, value ]

letString =
letBindings
|> List.map toLetString
|> String.join "\n\n"
in
if letBindings == [] then
if List.isEmpty letBindings then
""

else
Expand All @@ -30,4 +25,7 @@ generate letBindings =
let
{0}
in"""
[ letString ]
[ letBindings
|> List.map toLetString
|> String.join "\n\n"
]
6 changes: 1 addition & 5 deletions generator/src/Graphql/Parser/ClassCaseName.elm
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,4 @@ normalized (ClassCaseName rawName) =

isBuiltIn : ClassCaseName -> Bool
isBuiltIn (ClassCaseName rawName) =
if String.startsWith "__" rawName then
True

else
False
String.startsWith "__" rawName
11 changes: 6 additions & 5 deletions generator/tests/Parser/DecodeTests.elm
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ all =
|> Result.map Group.interfaceImplementorsDict
|> Expect.equal
(Ok
[ typeDefinition "Character"
([ typeDefinition "Character"
(InterfaceType
[ { name = CamelCaseName.build "id"
, description = Just "The id of the character."
Expand All @@ -525,7 +525,7 @@ all =
[]
)
Nothing
, typeDefinition "Biological"
, typeDefinition "Biological"
(InterfaceType
[ { name = CamelCaseName.build "id"
, description = Just "The id of the character."
Expand All @@ -537,7 +537,7 @@ all =
[ ClassCaseName.build "Character" ]
)
Nothing
, typeDefinition "Human"
, typeDefinition "Human"
(ObjectType
[ { name = CamelCaseName.build "id"
, description = Just "The id of the character."
Expand All @@ -548,7 +548,8 @@ all =
[ ClassCaseName.build "Biological", ClassCaseName.build "Character" ]
)
Nothing
]
|> Result.map Group.interfaceImplementorsDict
]
|> Group.interfaceImplementorsDict
)
)
]
Loading
Loading