-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make etlas.dhall the default configuration file for etlas #66
Comments
I manged to implement a initial version that uses
|
@jneira Can you share the output with |
Also can you add a debug log in |
@jneira Any particular reason for: jneira@5048002#diff-2d388570b233aa3bd5298e03db1e93c8L512 Shouldn't it be |
That was my first version but then etlas requires both files and throwed an error if etlas.dhall was not present. Maybe it should change to make it required only one of them. |
Oh right, that field is for required files. Then it looks ok. |
|
@jneira Maybe you can try populating |
Mmmm, that would be a solution but as i have to see how etlas does it maybe i can see where it does and hopefully fix in etlas itself, if possible |
Sure that works too! |
The problem seems to be in the initial parse of config files, comparing the |
Does filling in those fields properly fix the issue you were hitting? |
@rahulmutt yeah, i think dhall-to-etlas should fill it when parsing the file in this function. |
@jneira That workaround seems OK for now, let's just focus on correctness first and then we'll worry about performance. To fix that problem of building package twice, we need to do a filtering before returning with the Find all |
Hi, i have a initial working version of etlas with dhall integration
Pending:
|
This is great news @jneira! :D |
Performance of parsing dhall config files:
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 10.9980705 seconds
....
D:\ws\eta\eta-test>type etlas.dhall
./etlas.raw.dhall sha256:e036b394eb43c044f3ef6fbcbd92c4ddac88ad4ce2bf8c5ac2cbba66d0e23185
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 17.3785114 seconds
....
D:\ws\eta\eta-test>etlas clean -v3
....
Reading package configuration from .\etlas.dhall
Configuration readed in 0.1092007 seconds
.... So we must use the cached version whenever possible. The computation of the hash takes the same time that parsing itself (cause dhall caches the normalized version) so we should find out the way to get the content and the hash at same time, in one step. Then we should save the import hashed dhall file in |
Great analysis @jneira! That 17 seconds is a bit much. I think a performance target should be to get it down to at least 3 seconds - it's OK if it's a couple seconds slower than the Users will typically not change their .cabal files very often, but they shouldn't have to think twice to make changes b/c of perf issues! |
Well, another option is to use a derived |
After updating etlas to use
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 8.1900525 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 6.3024404 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 10.3272662 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning...
D:\ws\eta\eta-test>etlas clean -v3
no user package environment file found at D:\ws\eta\eta-test
Reading package configuration from .\etlas.dhall
Configuration readed in 0.1092007 seconds
Using internal setup method with build-type Simple and args:
["clean","--verbose=3","--builddir=dist"]
cleaning... Although times are better, i think the threshold should be 5 seconds for the worst case for being acceptable, that is, when user changes the |
Hi, i've done some tests with the new config files,
The unique test that fails is
Copying C:\Users\user\AppData\Roaming\etlas\packages\jneira\eta-test\0.2.0.0 to
C:\TEMP\etlas-tmp-8040...
copy directory
'C:\Users\user\AppData\Roaming\etlas\packages\jneira\eta-test\0.2.0.0' to
'C:\TEMP\etlas-tmp-8040'.
creating C:\TEMP\etlas-tmp-8040
creating C:\TEMP\etlas-tmp-8040\src
creating C:\TEMP\etlas-tmp-8040\java
creating C:\TEMP\etlas-tmp-8040\app
creating C:\TEMP\etlas-tmp-8040\.git
creating C:\TEMP\etlas-tmp-8040\.git\objects\fd
creating C:\TEMP\etlas-tmp-8040\.git\objects
creating C:\TEMP\etlas-tmp-8040\.git\objects\fd
creating C:\TEMP\etlas-tmp-8040\.git\objects\e1
creating C:\TEMP\etlas-tmp-8040\.git\objects\d0
creating C:\TEMP\etlas-tmp-8040\.git\objects\be
creating C:\TEMP\etlas-tmp-8040\.git\objects\ba
creating C:\TEMP\etlas-tmp-8040\.git\objects\b9
creating C:\TEMP\etlas-tmp-8040\.git\objects\b4
creating C:\TEMP\etlas-tmp-8040\.git\objects\7a
creating C:\TEMP\etlas-tmp-8040\.git\objects\6d
creating C:\TEMP\etlas-tmp-8040\.git\objects\63
creating C:\TEMP\etlas-tmp-8040\.git\objects\55
creating C:\TEMP\etlas-tmp-8040\.git\objects\51
creating C:\TEMP\etlas-tmp-8040\.git\objects\27
creating C:\TEMP\etlas-tmp-8040\.git\objects\24
creating C:\TEMP\etlas-tmp-8040\.git\objects\03
creating C:\TEMP\etlas-tmp-8040\.git\logs
creating C:\TEMP\etlas-tmp-8040\.git\info
creating C:\TEMP\etlas-tmp-8040\.git\hooks
Configuring eta-test-0.2.0.0...
Reading package configuration from C:\TEMP\etlas-tmp-8040\etlas.dhall
Configuration readed in 6.24004e-2 seconds
Writing derived cabal file from dhall file: dist\etlas.dhall.cabal
Using self-exec internal setup method with build-type Simple and args:
["act-as-setup","--build-type=Simple","--","configure","--verbose=3","--builddir
=dist","--eta","--prefix=C:\\Users\\user\\AppData\\Roaming\\etlas","--bindir=C:\\
Users\\user\\AppData\\Roaming\\etlas\\bin","--libdir=C:\\Users\\user\\AppData\\Roa
ming\\etlas","--libsubdir=eta-0.8.6.3\\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E",
"--dynlibdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\eta-0.8.6.3","--libexecdir
=C:\\Users\\user\\AppData\\Roaming\\etlas\\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9
E","--datadir=C:\\Users\\user\\AppData\\Roaming\\etlas","--datasubdir=eta-0.8.6.3
\\eta-test-0.2.0.0","--docdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\doc\\eta-
0.8.6.3\\eta-test-0.2.0.0","--htmldir=C:\\Users\\user\\AppData\\Roaming\\etlas\\d
oc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--haddockdir=C:\\Users\\user\\AppData\\
Roaming\\etlas\\doc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--etadocdir=C:\\Users
\\user\\AppData\\Roaming\\etlas\\doc\\eta-0.8.6.3\\eta-test-0.2.0.0\\html","--sys
confdir=C:\\Users\\user\\AppData\\Roaming\\etlas\\etc","--user","--ipid=eta-test-
0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E","--extra-prog-path=C:\\Users\\user\\AppData\\Roam
ing\\etlas\\bin","--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih","--dep
endency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih","--disable-tests","--exact-co
nfiguration","--disable-benchmarks","--cabal-file","dist\\etlas.dhall.cabal"]
D:\Trabajo\NO-SALVAR\jneira\bin\etlas.exe act-as-setup --build-type=Simple --
configure --verbose=3 --builddir=dist --eta
--prefix=C:\Users\user\AppData\Roaming\etlas
--bindir=C:\Users\user\AppData\Roaming\etlas\bin
--libdir=C:\Users\user\AppData\Roaming\etlas
--libsubdir=eta-0.8.6.3\eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E
--dynlibdir=C:\Users\user\AppData\Roaming\etlas\eta-0.8.6.3
--libexecdir=C:\Users\user\AppData\Roaming\etlas\eta-test-0.2.0.0-Ifk6o0gD3O9Bytx
AzLdR9E
--datadir=C:\Users\user\AppData\Roaming\etlas
--datasubdir=eta-0.8.6.3\eta-test-0.2.0.0
--docdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0
--htmldir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0\ht
ml
--haddockdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0
\html
--etadocdir=C:\Users\user\AppData\Roaming\etlas\doc\eta-0.8.6.3\eta-test-0.2.0.0\
html
--sysconfdir=C:\Users\user\AppData\Roaming\etlas\etc --user
--ipid=eta-test-0.2.0.0-Ifk6o0gD3O9BytxAzLdR9E
--extra-prog-path=C:\Users\user\AppData\Roaming\etlas\bin
--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih
--dependency=base=base-4.11.1.0-9g5RhDO8RwrLhMSXPi8Eih --disable-tests
--exact-configuration --disable-benchmarks --cabal-file dist\etlas.dhall.cabal
Redirecting build log to {handle:
C:\Users\user\AppData\Roaming\etlas\logs\eta-0.8.6.3\eta-test-0.2.0.0-Ifk6o0gD3O9
BytxAzLdR9E.log}
?callStack, called at .\Distribution\Compat\Stack.hs:45:13 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Compat.Stack
callStack, called at .\Distribution\Simple\Utils.hs:591:44 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
withCallStackPrefix, called at .\Distribution\Simple\Utils.hs:628:7 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
withMetadata, called at .\Distribution\Simple\Utils.hs:353:15 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple.Utils
die', called at .\Distribution\PackageDescription\Parsec.hs:84:7 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.PackageDescription.Parsec
readAndParseFile, called at .\Distribution\PackageDescription\Parsec.hs:96:33 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.PackageDescription.Parsec
readGenericPackageDescription, called at .\Distribution\Simple.hs:250:19 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
confPkgDescr, called at .\Distribution\Simple.hs:219:33 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
configureAction, called at .\Distribution\Simple.hs:184:19 in etlas_9Nqq0YQZEmg4GE9xabkO2B:Distribution.Simple
etlas.exe: Error Parsing: file "dist\etlas.dhall.cabal" doesn't exist. Cannot
continue.
etlas: Leaving directory 'C:\TEMP\etlas-tmp-8040' So the dhall file is parsed and used, the derived cabal file writed (not error in that step) but |
From a cursory glance at the logs, I notice that the dhall parsing code uses relative paths, so perhaps the derived cabal file is being written to the
directory structure
cabal.project
Run the following:
In this case, it should create the |
The performance of near version of dhall-to-etlas 1.4.0.0 starts to be reasonable:
😆 |
That's amazing @jneira! Thank for you all the work for making dhall practical :) |
With #83 we'll got the new performance improvements and the fix to |
The performance of etlas using dhall has improved and it is below 5 seconds:
So it is way better but not enough to use it without cache.
|
This sounds like a great idea.
Can you elaborate on this? So are you saying that it's taking 4.75 seconds because caching is happening in addition to parsing? w/o caching how much time would it take? |
The second option would be read the changed dhall file (in 4.75 s), start to build immediately in one thread and start another one to cache the readed data (that hopefully will end in 4 seconds aprox.) |
Finally we've implemented a cache of etlas.dhall using builtin dhall cache mechanism: #88 |
@rahulmutt has made good points about the usage of the dhall format in |
With #96 |
possible improvements
|
To make
|
@jneira Sounds like a nice idea. Do you think this would make the performance even better? |
Definitely it will for |
Perhaps the easiest way is to just add it to |
I've requested a pr to bump up dhall version to 1.23: #102 Reading and caching package configuration from dhall file: .\etlas.dhall
Configuration readed in 4.836031 seconds |
@jneira That's wonderful! What are the remaining tasks for this issue? |
Mmm, i think only left use a local copy of imports as suggested in #66 (comment) |
I've updated dhall support for 1.26.1 of dhall-to-etlas and etlas: #108 |
As described in typelead/eta#704 (comment).
Here we can track the progress and specific issues about. The plan would be:
cabal new-build
) work withetlas.dhall
as the default configuration fileetlas.dhall
if present and ${project-name}.cabal otherwisecabal build
) works in a similar waycheck, format (undocumented!), etcThe text was updated successfully, but these errors were encountered: