Use ctx.actions.args() in compile_scala #1124
Conversation
| if (lSplit.length > 1) { | ||
| hm.put(lSplit[0], lSplit[1]); | ||
| } | ||
| HashMap<String, String> hm = new HashMap<>(); |
There was a problem hiding this comment.
Is there a reason to stay here with a HashMap instead of Map?
There was a problem hiding this comment.
Probably not. Just left it as it was. So now I've changed var and return type to Map
| enable_diagnostics_report = enable_diagnostics_report, | ||
| diagnostics_output = diagnosticsfile.path, | ||
| ) | ||
| args = ctx.actions.args() |
There was a problem hiding this comment.
Can args collection be extracted into a separate function?
There was a problem hiding this comment.
Probably it can. But if I just extract whole args to separate function it will contain same huge parameter list as compile_scala. If I extract multiple functions to build args I'll lose visibility how final args file would look like. I'll look what I can do about it.
|
I'm sorry but I'm moving this PR back to draft mode. I noticed that |
|
Also can you please see what tests we’re missing?
On Thu, 22 Oct 2020 at 15:19 Simonas Pinevičius ***@***.***> wrote:
I'm sorry but I'm moving this PR back to draft mode. I noticed that
argfile was passed as input to the ctx.actions.run and with this change
it is not. I need to understand consequences of such change.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#1124 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKQQF4QNRVAESFLXRTKLP3SMAPL3ANCNFSM4S2ANA5A>
.
--
*Ittai Zeidman*
40 Hanamal street, Tel Aviv, Israel
<http://www.wix.com>
|
simuons
force-pushed
the
compile-scala-with-args
branch
from
eb07522
to
75bdebb
Compare
This should decrease memory footprint in analysis phase by - avoiding contatenation of strings - avoiding building list in memory - defering expansions of depsets as described in https://docs.bazel.build/versions/master/skylark/performance.html#use-ctxactionsargs-for-command-lines
simuons
force-pushed
the
compile-scala-with-args
branch
from
75bdebb
to
5f904b9
Compare
|
I'm sorry I had to do a rebase because it was handing for too long. There are no new tests because this is a completely internal refactoring. I believe that eventually we will need to move to |
Description
Use ctx.actions.args() to pass parameters to persistent worker.
Motivation
As per recommendations in https://docs.bazel.build/versions/master/skylark/performance.html#use-ctxactionsargs-for-command-lines this should reduce memory consumption at analysis phase by: