From 3e2478f85b4193fb67f4991a71e5d9051721e4c3 Mon Sep 17 00:00:00 2001 From: aminya Date: Mon, 6 Jan 2020 19:21:24 +0330 Subject: [PATCH] Addressing review comments v2.0.0 pull request bot pathof https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362529215 rename snoopiBenchBot ro snoopiBench https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362527787 Doc reference Fixes https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362350543 Naming convention =@. https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362465439 Benchmarking the infer time of the tests https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362516354 code quoting const UStrings https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362516641 https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362515357 file name typo https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362514140 Link updated https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362437563 typo https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362438281 __precompile__ https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362445178 Base.read and Base.write https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362441319 https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362441696 UString https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362439716 precompile include clarification https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362350592 Experimental https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362447726 Using isfile https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362466113 test dep Clarification https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362350332 and https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362350307 Comitter comment Fixes https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362350199 Base.write instead of (de)activated.jl in tests Fixes https://github.com/timholy/SnoopCompile.jl/pull/49#discussion_r362447423 --- docs/src/bot.md | 48 +++++++++++------- src/bot.jl | 18 +++---- src/bot/botutils.jl | 5 +- ...ecomileInclude.jl => precompileInclude.jl} | 46 +++++++++-------- src/bot/{snoopiBenchBot.jl => snoopiBench.jl} | 49 ++++++++++++------- src/bot/snoopiBot.jl | 6 +-- test/bot/activated.jl | 2 - test/bot/bot.jl | 30 ++++++++---- test/bot/deactivated.jl | 2 - 9 files changed, 115 insertions(+), 91 deletions(-) rename src/bot/{precomileInclude.jl => precompileInclude.jl} (69%) rename src/bot/{snoopiBenchBot.jl => snoopiBench.jl} (70%) delete mode 100644 test/bot/activated.jl delete mode 100644 test/bot/deactivated.jl diff --git a/docs/src/bot.md b/docs/src/bot.md index b2548dde5..af2b0fa0e 100644 --- a/docs/src/bot.md +++ b/docs/src/bot.md @@ -1,4 +1,4 @@ -# SnoopCompile Bot +# SnoopCompile Bot (EXPERIMENTAL) You can use SnoopCompile bot to automatically and continuously create precompile files. @@ -7,7 +7,7 @@ One should add 3 things to a package to make the bot work: ---------------------------------- -- workflow file: +- Workflow file: create a workflow file with this path in your repository `.github/workflows/SnoopCompile.yml` and use the following content: @@ -32,9 +32,9 @@ jobs: with: version: ${{ matrix.julia-version }} - name: Install dependencies - run: julia --project=@. -e 'using Pkg; Pkg.instantiate();' + run: julia --project -e 'using Pkg; Pkg.instantiate();' - name : Add SnoopCompile and current package - run: julia -e 'using Pkg; Pkg.add(PackageSpec(url = "https://github.com/aminya/SnoopCompile.jl", rev ="packageSnooper")); Pkg.develop(PackageSpec(; path=pwd()));' + run: julia -e 'using Pkg; Pkg.add("SnoopCompile"); Pkg.develop(PackageSpec(; path=pwd()));' - name: Install Test dependencies run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()' - name: Generating precompile files @@ -44,36 +44,37 @@ jobs: # https://github.com/marketplace/actions/create-pull-request - name: Create Pull Request - uses: peter-evans/create-pull-request@v2-beta + uses: peter-evans/create-pull-request@v2.0.0 with: token: ${{ secrets.GITHUB_TOKEN }} commit-message: Update precompile_*.jl file - committer: Amin Yahyaabadi + committer: YOUR NAME # Change `committer` to your name and your email. title: '[AUTO] Update precompile_*.jl file' labels: SnoopCompile branch: create-pull-request/SnoopCompile - name: Check output environment variable run: echo "Pull Request Number - ${{ env.PULL_REQUEST_NUMBER }}" ``` -If your examples or tests have dependencies, you should add a `Test.toml` to your test folder. - -`Install Test dependencies` step is only needed if you have test dependencies other than Test. Otherwise, you should comment it. +`Install Test dependencies` step is only needed if you have test dependencies other than Test. Otherwise, you should comment it. In this case, if your examples or tests have dependencies, you should add a `Test.toml` to your test folder. ```yaml -# - name: Install Test dependencies -# run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()' +- name: Install Test dependencies + run: julia -e 'using SnoopCompile; SnoopCompile.addtestdep()' ``` For example for MatLang package: [Link](https://github.com/juliamatlab/MatLang/blob/master/.github/workflows/SnoopCompile.yml) -Change `committer` to your name and your email. - ---------------------------------- -- Add a `snoopCompile.jl` file under `deps/SnoopCompile`. The content of the file can be the examples that call the package functions: +- Precompile script + +Add a `snoopCompile.jl` file under `deps/SnoopCompile`. The content of the file should be a script that "exercises" the functionality you'd like to precompile. One option is to use your package's `"runtests.jl"` file, or you can write a custom script for this purpose. + + +For example, some examples that call the functions: ```julia using SnoopCompile @@ -88,7 +89,7 @@ end ``` [Ref]( https://github.com/juliamatlab/MatLang/blob/master/deps/SnoopCompile/snoopCompile.jl) -or If you do not have additional examples, you can use your runtests.jl file: +or if you do not have additional examples, you can use your runtests.jl file using this syntax: ```julia using SnoopCompile @@ -97,9 +98,15 @@ using SnoopCompile @snoopiBot "MatLang" ``` +[Also look at this](https://timholy.github.io/SnoopCompile.jl/stable/snoopi/#Precompile-scripts-1) + ---------------------------------- -- Two lines of code that includes the precompile file in your main module. It is better to have these lines commented, if you want to continuously develop and change your package offline. Only merging pull requests online will uncomment the lines automatically: +- Include precompile signatures + +Two lines of (commented) code that includes the precompile file in your main module. + +It is better to have these lines commented to continuously develop and change your package offline. snoopiBot will find these lines of code and will uncomment them in the created pull request. If they are not commented the bot will leave it as is in the pull request: ```julia # include("../deps/SnoopCompile/precompile/precompile_MatLang.jl") @@ -109,6 +116,9 @@ using SnoopCompile [Ref](https://github.com/juliamatlab/MatLang/blob/072ff8ed9877cbb34f8583ae2cf928a5df18aa0c/src/MatLang.jl#L26) +---------------------------------- + + ## Benchmark To measure the effect of adding precompile files. Add a `snoopBenchmark.jl`. The content of this file can be the following: @@ -117,14 +127,14 @@ Benchmarking the load infer time ```julia println("loading infer benchmark") -@snoopiBenchBot "MatLang" using MatLang +@snoopiBench "MatLang" using MatLang ``` Benchmarking the example infer time ```julia println("examples infer benchmark") -@snoopiBenchBot "MatLang" begin +@snoopiBench "MatLang" begin using MatLang examplePath = joinpath(dirname(dirname(pathof(MatLang))), "examples") # include(joinpath(examplePath,"Language_Fundamentals", "usage_Entering_Commands.jl")) @@ -135,7 +145,7 @@ end Benchmarking the tests: ```julia -@snoopiBenchBot "MatLang" +@snoopiBench "MatLang" ``` [Ref](https://github.com/juliamatlab/MatLang/blob/master/deps/SnoopCompile/snoopBenchmark.jl) diff --git a/src/bot.jl b/src/bot.jl index 09ee81622..61e16e497 100644 --- a/src/bot.jl +++ b/src/bot.jl @@ -1,18 +1,18 @@ -export precompileActivator, precompileDeactivator, precompilePather, @snoopiBot, @snoopiBenchBot, BotConfig +export precompile_activator, precompile_deactivator, precompile_pather, @snoopiBot, @snoopiBench, BotConfig -UStrings = Union{AbstractString,Regex,AbstractChar} +const UStrings = Union{AbstractString,Regex,AbstractChar} ################################################################ """ BotConfig -Config object that holds the options and configuration for the snoopCompile bot. This object is fed to the `@snoopiBot`. +Config object that holds the options and configuration for the SnoopCompile bot. This object is fed to the `@snoopiBot`. # Arguments: -- packageName::String -- subst::Vector{Pair{UStrings, UStrings}} : to replace a packages precompile setences with another's package like ["ImageTest" => "Images"] -- blacklist::Vector{UStrings} : to remove some precompile sentences +- `packageName::String` +- `subst::Vector{Pair{UStrings, UStrings}}` : to replace a packages precompile setences with another's package like `["ImageTest" => "Images"]` +- `blacklist::Vector{UStrings}` : to remove some precompile sentences -# Ustrings == Union{AbstractString,Regex,AbstractChar} # every string like type that replace() has a method for. +`const UStrings == Union{AbstractString,Regex,AbstractChar}` # every string like type that `replace()` has a method for. """ struct BotConfig packageName::String @@ -25,6 +25,6 @@ function BotConfig(packageName::String; subst::Vector{Pair{T1, T2}} where {T1<:U end include("bot/botutils.jl") -include("bot/precomileInclude.jl") +include("bot/precompileInclude.jl") include("bot/snoopiBot.jl") -include("bot/snoopiBenchBot.jl") +include("bot/snoopiBench.jl") diff --git a/src/bot/botutils.jl b/src/bot/botutils.jl index 79dc8bf74..c99f3eea0 100644 --- a/src/bot/botutils.jl +++ b/src/bot/botutils.jl @@ -6,10 +6,9 @@ Should be removed once Pkg allows adding test dependencies to the current enviro Used in Github Action workflow yaml file """ function addtestdep() - local testToml - try + if isfile("test/Test.toml") testToml = Pkg.Types.parse_toml("test/Test.toml") - catch + else error("please add a Test.toml to the /test directory for test dependencies") end diff --git a/src/bot/precomileInclude.jl b/src/bot/precompileInclude.jl similarity index 69% rename from src/bot/precomileInclude.jl rename to src/bot/precompileInclude.jl index 62bcbf914..ef63849db 100644 --- a/src/bot/precomileInclude.jl +++ b/src/bot/precompileInclude.jl @@ -1,26 +1,26 @@ """ - precompilePather(packageName::String) + precompile_pather(packageName::String) To get the path of precompile_packageName.jl file Written exclusively for SnoopCompile Github actions. # Examples ```julia -precompilePath, precompileFolder = precompilePather("MatLang") +precompilePath, precompileFolder = precompile_pather("MatLang") ``` """ -function precompilePather(packageName::String) +function precompile_pather(packageName::String) return "\"../deps/SnoopCompile/precompile/precompile_$packageName.jl\"", "$(pwd())/deps/SnoopCompile/precompile/" end -precompilePather(packageName::Symbol) = precompilePather(string(packageName)) -precompilePather(packageName::Module) = precompilePather(string(packageName)) +precompile_pather(packageName::Symbol) = precompile_pather(string(packageName)) +precompile_pather(packageName::Module) = precompile_pather(string(packageName)) ################################################################ -function precompileRegex(precompilePath) +function precompile_regex(precompilePath) # https://stackoverflow.com/questions/3469080/match-whitespace-but-not-newlines # {1,} for any number of spaces c1 = Regex("#[^\\S\\r\\n]{0,}include\\($(precompilePath)\\)") @@ -32,19 +32,19 @@ end ################################################################ """ - precompileActivator(packagePath, precompilePath) + precompile_activator(packagePath, precompilePath) Activates precompile of a package by adding or uncommenting include() of *.jl file generated by SnoopCompile and _precompile_(). +packagePath is the same as `pathof`. However, `pathof(module)` isn't used to prevent loadnig the package. + Written exclusively for SnoopCompile Github actions. """ -function precompileActivator(packagePath::String, precompilePath::String) +function precompile_activator(packagePath::String, precompilePath::String) - file = open(packagePath,"r") - packageText = Base.read(file, String) - close(file) + packageText = Base.read(packagePath, String) - c1, c2, a1, a2 = precompileRegex(precompilePath) + c1, c2, a1, a2 = precompile_regex(precompilePath) # Checking availability of _precompile_ code commented = occursin(c1, packageText) && occursin(c2, packageText) @@ -58,9 +58,8 @@ function precompileActivator(packagePath::String, precompilePath::String) ), init = packageText) - file = open(packagePath,"w") - Base.write(file, packageEdited) - close(file) + Base.write(packagePath, packageEdited) + println("precompile is activated") elseif available # do nothing @@ -76,19 +75,19 @@ function precompileActivator(packagePath::String, precompilePath::String) end """ - precompileDeactivator(packagePath, precompilePath) + precompile_deactivator(packagePath, precompilePath) Deactivates precompile of a package by commenting include() of *.jl file generated by SnoopCompile and _precompile_(). +packagePath is the same as `pathof`. However, `pathof(module)` isn't used to prevent loadnig the package. + Written exclusively for SnoopCompile Github actions. """ -function precompileDeactivator(packagePath::String, precompilePath::String) +function precompile_deactivator(packagePath::String, precompilePath::String) - file = open(packagePath,"r") - packageText = Base.read(file, String) - close(file) + packageText = Base.read(packagePath, String) - c1, c2, a1, a2 = precompileRegex(precompilePath) + c1, c2, a1, a2 = precompile_regex(precompilePath) # Checking availability of _precompile_ code commented = occursin(c1, packageText) && occursin(c2, packageText) @@ -102,9 +101,8 @@ function precompileDeactivator(packagePath::String, precompilePath::String) ), init = packageText) - file = open(packagePath,"w") - Base.write(file, packageEdited) - close(file) + Base.write(packagePath, packageEdited) + println("precompile is deactivated") elseif commented # do nothing diff --git a/src/bot/snoopiBenchBot.jl b/src/bot/snoopiBench.jl similarity index 70% rename from src/bot/snoopiBenchBot.jl rename to src/bot/snoopiBench.jl index a6c5e9a4b..1b07418ef 100644 --- a/src/bot/snoopiBenchBot.jl +++ b/src/bot/snoopiBench.jl @@ -2,12 +2,20 @@ """ timesum(snoop) -Calculates and prints the total time measured by a snoop macro. It is used inside @snoopiBenchBot. +Calculates and prints the total time measured by a snoop macro. -Julia can cache inference results so to measure the effect of adding _precompile_() sentences generated by snoopi to your package, use the [`@snoopiBenchBot`](@ref). This benchmark measures inference time taken during loading and running of a package. +It is used inside @snoopiBench. Julia can cache inference results so to measure the effect of adding _precompile_() sentences generated by snoopi to your package, use the [`@snoopiBench`](@ref). This benchmark measures inference time taken during loading and running of a package. +# Examples +```julia +using SnoopCompile +data = @snoopi begin + include(joinpath(dirname(dirname(pathof(MatLang))),"test","runtests.jl")) +end; +println(timesum(data)); +``` -## Manually benchmarking withtout using [`@snoopiBenchBot`](@ref) +## Manual Benchmark (withtout using [`@snoopiBench`](@ref)) - dev your package - comment the precompile part of your package (`include()` and `_precompile_()`) @@ -46,23 +54,23 @@ end ################################################################ """ - @snoopiBenchBot(packageName::String, snoopScript::Expr) - @snoopiBenchBot(packageName::String) + @snoopiBench(packageName::String, snoopScript::Expr) + @snoopiBench(packageName::String) -Performs an infertime benchmark by activating and deactivating the __precompile__() +Performs an infertime benchmark by activating and deactivating the _precompile_() # Examples Benchmarking the load infer time ```julia println("loading infer benchmark") -@snoopiBenchBot "MatLang" using MatLang +@snoopiBench "MatLang" using MatLang ``` Benchmarking the example infer time ```julia println("examples infer benchmark") -@snoopiBenchBot "MatLang" begin +@snoopiBench "MatLang" begin using MatLang examplePath = joinpath(dirname(dirname(pathof(MatLang))), "examples") # include(joinpath(examplePath,"Language_Fundamentals", "usage_Entering_Commands.jl")) @@ -70,17 +78,12 @@ println("examples infer benchmark") include(joinpath(examplePath,"Language_Fundamentals", "Data_Types", "usage_Numeric_Types.jl")) end ``` - -Benchmarking the tests: -```julia -@snoopiBenchBot "MatLang" -``` """ -macro snoopiBenchBot(packageName::String, snoopScript::Expr) +macro snoopiBench(packageName::String, snoopScript::Expr) ################################################################ packagePath = joinpath(pwd(),"src","$packageName.jl") - precompilePath, precompileFolder = precompilePather(packageName) + precompilePath, precompileFolder = precompile_pather(packageName) juliaCode = """ using SnoopCompile; data = @snoopi begin @@ -101,14 +104,14 @@ macro snoopiBenchBot(packageName::String, snoopScript::Expr) println("""Precompile Deactivated Benchmark ------------------------ """) - precompileDeactivator($packagePath, $precompilePath); + precompile_deactivator($packagePath, $precompilePath); ### Log the compiles run($juliaCmd) ################################################################ println("""Precompile Activated Benchmark ------------------------ """) - precompileActivator($packagePath, $precompilePath) + precompile_activator($packagePath, $precompilePath) ### Log the compiles run($juliaCmd) println("""******************* @@ -119,7 +122,15 @@ macro snoopiBenchBot(packageName::String, snoopScript::Expr) end -macro snoopiBenchBot(packageName::String) +""" + @snoopiBench packageName::String + +Benchmarking the infer time of the tests: +```julia +@snoopiBench "MatLang" +``` +""" +macro snoopiBench(packageName::String) package = Symbol(packageName) snoopScript = :( using $(package); @@ -127,6 +138,6 @@ macro snoopiBenchBot(packageName::String) include(runtestpath); ) return quote - @snoopiBenchBot $packageName $(snoopScript) + @snoopiBench $packageName $(snoopScript) end end diff --git a/src/bot/snoopiBot.jl b/src/bot/snoopiBot.jl index 8aaee61d7..4f24712a9 100644 --- a/src/bot/snoopiBot.jl +++ b/src/bot/snoopiBot.jl @@ -28,14 +28,14 @@ macro snoopiBot(config::BotConfig, snoopScript) subst = config.subst ################################################################ packagePath = joinpath(pwd(),"src","$packageName.jl") - precompilePath, precompileFolder = precompilePather(packageName) + precompilePath, precompileFolder = precompile_pather(packageName) quote packageSym = Symbol($packageName) ################################################################ using SnoopCompile ################################################################ - precompileDeactivator($packagePath, $precompilePath); + precompile_deactivator($packagePath, $precompilePath); ################################################################ ### Log the compiles @@ -49,7 +49,7 @@ macro snoopiBot(config::BotConfig, snoopScript) onlypackage = Dict( packageSym => sort(pc[packageSym]) ) SnoopCompile.write($precompileFolder,onlypackage) ################################################################ - precompileActivator($packagePath, $precompilePath) + precompile_activator($packagePath, $precompilePath) end end diff --git a/test/bot/activated.jl b/test/bot/activated.jl deleted file mode 100644 index 59860213a..000000000 --- a/test/bot/activated.jl +++ /dev/null @@ -1,2 +0,0 @@ -include("../deps/SnoopCompile/precompile/precompile_TestPackage.jl") -_precompile_() diff --git a/test/bot/bot.jl b/test/bot/bot.jl index aa4f98cc9..a2adda0b9 100644 --- a/test/bot/bot.jl +++ b/test/bot/bot.jl @@ -3,20 +3,30 @@ using SnoopCompile, Test, Suppressor cd(@__DIR__) @testset "bot" begin @testset "precompileInclude" begin - @testset "precompilePather" begin - precompilePath, precompileFolder = precompilePather("TestPackage") + @testset "precompile_pather" begin + precompilePath, precompileFolder = precompile_pather("TestPackage") @test precompilePath == "\"../deps/SnoopCompile/precompile/precompile_TestPackage.jl\"" @test precompileFolder == "$(pwd())/deps/SnoopCompile/precompile/" end - @testset "precompileActivator" begin - precompilePath, precompileFolder = precompilePather("TestPackage") - @test (@capture_out precompileActivator("activated.jl", precompilePath)) == "precompile is already activated\n" + @testset "precompile_activator" begin + Base.write("activated.jl", """ + include("../deps/SnoopCompile/precompile/precompile_TestPackage.jl") + _precompile_() + """) + + precompilePath, precompileFolder = precompile_pather("TestPackage") + @test (@capture_out precompile_activator("activated.jl", precompilePath)) == "precompile is already activated\n" end - @testset "precompileDeactivator" begin - precompilePath, precompileFolder = precompilePather("TestPackage") - @test (@capture_out precompileDeactivator("deactivated.jl", precompilePath)) == "precompile is already deactivated\n" + @testset "precompile_deactivator" begin + Base.write("deactivated.jl", """ + # include("../deps/SnoopCompile/precompile/precompile_TestPackage.jl") + # _precompile_() + """) + + precompilePath, precompileFolder = precompile_pather("TestPackage") + @test (@capture_out precompile_deactivator("deactivated.jl", precompilePath)) == "precompile is already deactivated\n" end end @@ -40,13 +50,13 @@ cd(@__DIR__) end end - @testset "snoopiBenchBot" begin + @testset "snoopiBench" begin using Pkg; Pkg.develop("MatLang") examplePath = Base.read(`cmd /c julia -e 'import MatLang; print(pathof(MatLang))'`, String) cd(dirname(dirname(examplePath))) - @snoopiBenchBot "MatLang" begin + @snoopiBench "MatLang" begin using MatLang examplePath = joinpath(dirname(dirname(pathof(MatLang))), "examples") # include(joinpath(examplePath,"Language_Fundamentals", "usage_Entering_Commands.jl")) diff --git a/test/bot/deactivated.jl b/test/bot/deactivated.jl deleted file mode 100644 index 819961cd6..000000000 --- a/test/bot/deactivated.jl +++ /dev/null @@ -1,2 +0,0 @@ -# include("../deps/SnoopCompile/precompile/precompile_TestPackage.jl") -# _precompile_()