path.exists() causes a massive slowdown on Windows

[dvtomas]

This line https://github.com/sile/sloggers/blob/153c00a59f7218c1d96f522fb7a95c80bb0d530c/src/file.rs#L228

is called on every write into a file. The path.exists syscall is probably very slow on Windows, it brings the logging performance to its knees, only a few tens of lines per second get logged. I tried a quick hack and removed it and the performance went up massively, but I guess the proper solution will not be as simple as just removing it. But something definitely needs to be done about this, maybe checking the result of write! and only if that fails, call reopen_if_needed() or something like that.

While somebody is at this, it could perhaps be a good idea to also wrap the file in a BufWriter to enhance the performance even more?

[sile]

Thank you for reporting the problem.

maybe checking the result of write! and only if that fails, call reopen_if_needed() or something like that.

On Unix environments, write operations to an opened file does not fail even if the file has been removed after the opening. So, on Windows and Unix, it may be needed to use different implementations for detecting the necessity of reopening.

it could perhaps be a good idea to also wrap the file in a BufWriter to enhance the performance even more?

I think that's a good idea.

[dvtomas]

Ok. I guess if somebody intentionally deletes the log file, maybe we can assume that he knows what he is doing and not care for that case? And the regular case when reopen_if_needed is called as an action to recover from rotate() is already handled in rotate if I understand the sources correctly.. So perhaps, under these assumptions, just removing the reopen_if_needed call from write could actually be a good solution (provided unit tests are written to assert correctness)?

[sile]

Typically, reopen_if_needed is needed for the case that log rotation is executed by an external program (e.g., logrotate that provides more flexible configurations than sloggers). In that case, on Unix, removing reopen_if_needed will cause log message omissions once a rotation is performed (by an external program).

[dvtomas]

I see, that's a good argument.

In that case, maybe checking for reopen_if_needed could be limited to be performed at most - let's say - once per a second?

If logrotate moves the file to a rotated file, that's not a problem - writes for up to one second will end up in the old file (now newly named, but technically the same inode), so no information is lost. After at most one second reopen_if_needed check will do it's thing.

If user intentionally deletes a file, he's probably content with losing some data :-) And after at most one second, he'll begin receiving new data.

What do you think?

[sile]

In that case, maybe checking for reopen_if_needed could be limited to be performed at most - let's say - once per a second?

I think it is a good way.

[dvtomas]

OK, I've created a PR