Skip to content

Comments

use ctRegex + avoid array allocation#196

Merged
AndrejMitrovic merged 1 commit intodlang:masterfrom
wilzbach:ctregex-no-array
Aug 28, 2016
Merged

use ctRegex + avoid array allocation#196
AndrejMitrovic merged 1 commit intodlang:masterfrom
wilzbach:ctregex-no-array

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Aug 28, 2016

These are two simple gotchas that I found during #195 and are independent to it or any refactoring.
Btw interestingly the compilation increases about 0.1s, but for the optimized build it decreases for nearly 0.2s

Compilation

command compilation time
dmd rdmd-master.d 1.21s
dmd rdmd.d 1.31s
dmd -O rdmd-master.d 1.96s
dmd -O rdmd.d 1.77s
echo rdmd_master.d rdmd.d "-O rdmd" | tr ' ' '\n' | parallel -j1 "echo {}; python -m timeit -n 5 -r 3 -s 'import os' 'os.system(\"dmd {}\")'"

Run time

With the used at #191

command compilation time
rdmd-master 607ms
rdmd-ct 605ms
rdmd-master-opt 598ms
rdmd-ct-opt 557ms

For a simple file with only two imports

command compilation time
rdmd-master 135ms
rdmd-ct 133ms
rdmd-master-opt 124ms
rdmd-ct-opt 111ms

For the benchmarks the following command was used :

echo rdmd-master rdmd-ct rdmd-master-opt rdmd-ct-opt | tr ' ' '\n' | parallel -j1 "echo {}; python -m timeit -n 10 -r 3 -s 'import os' 'os.system(\"./{} --force -main t1.d > /dev/null 2>&1\")'"

edit: rdmd-master = commit 1260719

@dlang-bot
Copy link
Contributor

@wilzbach, thanks for your PR! By analyzing the annotation information on this pull request, we identified @andralex, @CyberShadow and @leandro-lucarella-sociomantic to be potential reviewers. @andralex: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.

(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)


// Have at it
if (chain(root.only, myDeps.byKey).array.anyNewerThan(lastBuildTime))
if (chain(root.only, myDeps.byKey).anyNewerThan(lastBuildTime))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use root.only.chain.myDeps.byKey.anyNewerThan(lastBuildTime) here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it is much harder to read than current version. I'd even go as far as to say that chain should never be used with UFCS unless it is used to append to already existing pipeline.

@AndrejMitrovic AndrejMitrovic merged commit 6f92f7f into dlang:master Aug 28, 2016
@wilzbach wilzbach deleted the ctregex-no-array branch August 29, 2016 00:27
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

Successfully merging this pull request may close these issues.

5 participants