Incorrect, or at least highly confusing, wording regarding BufRead::lines iterator / in the “File I/O-read_lines” example

[steffahn]

The sentence introduced (alongside many good changes) in #1679 (cc @pringshia) that

We also update read_lines to return an iterator instead of allocating new String objects in memory for each line.

https://doc.rust-lang.org/nightly/rust-by-example/std_misc/file/read_lines.html

is wrong or at least highly confusing. I would understand this as claiming that the need for creating a new allocation for each line (for the Strings) is avoided, which is certainly not the case. The only thing that is avoided is

• the Vec's allocation for holding all the Strings
• the need to keep multiple Strings in memory at the same time (so at the end, the allocator might decide to re-use some/most of the memory)
• (and of course, the memory consumption of the initial String holding the whole file, from read_to_string, is avoided in the improved version of the code, but that's already mentioned in a separate sentence anyways)

Also, I fell in general, there could be a better explanation of what kind of efficiency is gained. The main gain here, in my view, is keeping memory consumption low; to more effectively improve run time it would be necessary to avoid creating all those Strings at all. There also seems to be the possibility that readers interpret this code as optimal in some sense. This already got better by no longer calling it “efficient method” but just “more efficient method”, but at least a remark that there are ways to become even significantly more efficient – at least in use-cases that admit re-using a String buffer – could be useful.

This issue came up during / is motivated by, the discussion in https://users.rust-lang.org/t/why-using-the-read-lines-iterator-is-much-slower-than-using-read-line/92815

[marioidival]

Hi, I'm closing this issue just to clean up the items that already exist. If you think this issue makes sense to stay open, please create a new issue with updated information and mention this one.

thanks!

[kpreid]

@marioidival Why does it make sense to close issues that are not fixed, especially marking them as "completed" status suggesting they've been fixed? Such bulk closure destroys valuable slowly-accumulated information about possible improvements to the documentation, especially as the original observer of a problem may not be actively monitoring their issue any more.

Please reopen this issue because it is not fixed.

[marioidival]

@kpreid If the message I posted when I closed this ticket didn't make sense, I'll try to make it a little clearer.

I'm doing this as a way to remove items that seem to hinder the current RBE improvement. For me (and by that I mean, my control), this ticket is not lost. I'm making a selection where I'm leaving tickets potentially available for other contributors to resolve, while others that seem outdated or even resolved but without closing the ticket would be cleaned and still, giving the possibility of me being wrong while making the decision to close it, it is reopened in an updated manner.

Do you understand my reasoning?

[kpreid]

while others that seem outdated or even resolved but without closing the ticket would be cleaned and still, giving the possibility of me being wrong while making the decision to close it, it is reopened in an updated manner.

Ah, I understand better your intent. I had thought that you were closing issues because they were old, not just because they were obsolete. That's an unfortunately common practice which discards information and hurts feelings, so I felt it was necessary to speak up. This sort of bulk closure with a generic comment is commonly done by corporate-run issue trackers and the message people have learned to take from it is “We're not going to fix old bugs unless we get yelled at really loudly”. That's quite discouraging, and what you were doing sounded an awful lot like it.

But, concretely, this specific issue is neither outdated nor fixed — the text has the same problem as when it was originally filed. In addition, you have also closed other issues that are valid (no change has been made since they filed):

• 15.3.2 "Aliasing" title confusion #1652
• 3.3 constants, it not clear #1655

There are probably more, but I have not done a thorough search. Please re-open these issues. Requiring people to re-file the issue discourages contributors by giving them make-work, discards information because not everyone is going to re-file, and gives no benefit — the text of the issue is just as accurate as it was filed.

[steffahn]

I can confirm that the closing message doesn't appear particularly powerful in concinving/motivating people to actually open a new issue. I wouldn't feel compelled to open a new issue. It sure sounded like old issues are just closed... even though this issue isn't even all that old. With that context, regarding re-opening a new issue, I would just think “What's the point? What's the prospect? Copy paste everything, only for it to be closed again less than one year from now?”

I can also confirm that marking the issue as completed is highly counterproductive. Glancing at the email notification I received, the latest and most prominently displayed only saying “Closed #ISSUE_NUMBER as completed”, I had only perceived this as “neat, some issue in Rust-by-example that I had reported got fixed”. Without @kpreid's reply I might never have noticed what this was about.

[marioidival]

@steffahn @kpreid Ok, Live and Learn 😅

I didn't do this with the intention of being bad to you. For me, dealing with tickets this way made sense, but I will think of a better approach.

Thank you for letting me know.