fix: allow for stdout and stderr streams to be unbufferred directly.

[sriram-mv]

Issue #, if available:

• N/A ( I can create one and reference it)

Description of changes:

• Allow for stdout and stderr streams to be unbuffered by default.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[sanathkr]

What are the performance impacts of making the streams unbuffered? How does this work/degrade in Windows cmd prompt and other non-iterm like terminals? How about through a SSH terminal?

I'd like us to be a bit thorough before enabling it globally.. I am okay if this is selectively added only when debugging because I know we might block the process for too long on sockets at that point..

[sanathkr]

can we do this selectively only for debugging?

[jfuss]

For all of debugging? Can we scope this further down than that?

[sriram-mv]

we can scope it down to say "if you specify a debug port", I think its a fair trade off for performance.

[sriram-mv]

Yeah we can add it to debugging mode. That works, in terms of comparison, can we run the integration tests with a timer with and without this change on every platform and verify performance impact? Will that work?

…

On Wed, Oct 10, 2018 at 5:20 PM, Sanath Kumar Ramesh < ***@***.***> wrote: ***@***.**** requested changes on this pull request. What are the performance impacts of making the streams unbuffered? How does this work/degrade in Windows cmd prompt and other non-iterm like terminals? How about through a SSH terminal? I'd like us to be a bit thorough before enabling it globally.. I am okay if this is selectively added only when debugging because I know we might block the process for too long on sockets at that point.. ------------------------------ In samcli/cli/main.py <#708 (comment)>: > @click.command(cls=BaseCommand) ***@***.***_streams can we do this selectively only for debugging? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#708 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADmJlnZDV4V1ersQrQ5woi1OfKbKC4Qtks5ujo7dgaJpZM4XWe1o> .

[sriram-mv]

So I ran the integration tests with and without this commit on the local invoke path, and found no difference in timings.

Without this commit.

(aws_sam-sN63_SKq) srirammv@localrainbow:16:20:10:~/aws_sam_cli_WS/aws-sam-cli$time pytest tests/integration/local/invoke/
=============================================================================================================== test session starts ================================================================================================================
platform darwin -- Python 3.6.5, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
rootdir: /Users/srirammv/aws_sam_cli_WS/aws-sam-cli, inifile: pytest.ini
plugins: cov-2.4.0
collected 14 items

tests/integration/local/invoke/test_integrations_cli.py ............
tests/integration/local/invoke/runtimes/test_with_runtime_zips.py ..

============================================================================================================ 14 passed in 55.47 seconds ============================================================================================================

real	0m56.009s
user	0m12.071s
sys	0m2.177s

With commit

(aws_sam-sN63_SKq) srirammv@localrainbow:16:18:38:~/aws_sam_cli_WS/aws-sam-cli$time pytest tests/integration/local/invoke/
=============================================================================================================== test session starts ================================================================================================================
platform darwin -- Python 3.6.5, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
rootdir: /Users/srirammv/aws_sam_cli_WS/aws-sam-cli, inifile: pytest.ini
plugins: cov-2.4.0
collected 14 items

tests/integration/local/invoke/test_integrations_cli.py ............
tests/integration/local/invoke/runtimes/test_with_runtime_zips.py ..

============================================================================================================ 14 passed in 53.78 seconds ============================================================================================================

real	0m54.262s
user	0m11.396s
sys	0m2.190s

Also setting os.environ will only matter for that session of the invoke.

[sanathkr]

Makes more sense now. Thanks!

[sanathkr]

Actually on second thought, can you skip the setting of "0" to facilitate usage of this class in multi-threaded scenarios. If you do

if self.debug_port:
  os.environ[...] = 1

then you don't have the race condition to worry about.

[sanathkr]

Thanks!