Skip to content
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

[env_logger] Adds a newline to format string before writing #219

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@tailhook
Copy link

tailhook commented Aug 14, 2017

This is important so in unix systems most of the time buffer is output
atomically (until certain length of message). Which means you don't get
two lines squashed together if they are written simultaneously.

I believe most of the time it's doesn't influence performance, because
underlying buffer of string is probably overallocated a bit anyway.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Aug 15, 2017

Stdout is line buffered - when will this change behavior?

@tailhook

This comment has been minimized.

Copy link
Author

tailhook commented Aug 15, 2017

Stdout is line buffered - when will this change behavior?

Is it? I observe broken logs every time I start two processes. Logs are written to stderr by default, though. Is there any difference between stdout and stderr?

@tailhook

This comment has been minimized.

Copy link
Author

tailhook commented Aug 15, 2017

Well, it looks like undocumented, but yes, stdout is line buffered whereas stderr isn't. So we could revert to the old behavior on stdout.

[env_logger] Adds a newline to format string before writing
This is important so in unix systems most of the time buffer is output
atomically (until certain length of message). Which means you don't get
two lines squashed together if they are written simultaneously.

I believe most of the time it's doesn't influence performance, because
underlying buffer of string is probably overallocated a bit anyway.

@tailhook tailhook force-pushed the tailhook:atomic_newline branch from 6db2230 to 2094be4 Aug 15, 2017

@tailhook

This comment has been minimized.

Copy link
Author

tailhook commented Aug 15, 2017

Done. Also fixed bug. (I used to think that it's enough to run tests in root directory rather than env)

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Sep 17, 2017

Thanks for this @tailhook! Do you mind rebasing this over at https://github.com/sebasmagri/env_logger, which is the new home for env_logger?

@KodrAus KodrAus referenced this pull request Sep 17, 2017

Closed

Remove env_logger from this repository #145

2 of 2 tasks complete
@tailhook

This comment has been minimized.

Copy link
Author

tailhook commented Sep 17, 2017

Well, it's not so useful after: sebasmagri/env_logger#19

I'm not sure what to do:

  1. Now this behavior depends on actual formatter. Default one is wrong at this respect, and also a bad example to do.
  2. On the other hand, if default formatter fixed it makes the aformentioned PR useless?
@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Sep 22, 2017

We've started discussing this over in sebasmagri/env_logger#22 so I'll close this PR now. I'd definitely like to come up with a good solution to this before a 1.0 of env_logger!

@KodrAus KodrAus closed this Sep 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.