Conversion from UTF32 to UTF8 for review
in [MFC]
|
From: Peter Olcott on 1 Jun 2010 09:51 On 5/31/2010 9:16 PM, Joseph M. Newcomer wrote: > See below... > On Mon, 31 May 2010 20:16:53 +0200, "Giovanni Dicanio" > <giovanniDOTdicanio(a)REMOVEMEgmail.com> wrote: > >> "Joseph M. Newcomer"<newcomer(a)flounder.com> wrote: >> >>>> UTF8.reserve(UTF32.size() * 4); // worst case >>> **** >>> Note that this will call malloc(), which will involve setting a lock, then >>> searching for a >>> block to allocate, then releasing the lock. Since you have been a fanatic >>> about >>> performance, why is it you put a very expensive operation like 'reserve' >>> in your code? >>> >>> While it is perfectly reasonable, it seems inconsistent with your >>> previously-stated goals. >> >> Joe: I'm not sure if you are ironic or something :) ... but I believe that >> std::vector::reserve() with a proper capacity value, followed by several >> push_back()s, is very efficient. >> Sure, not as efficient as a static stack-allocated array, but very >> efficient. > **** > But this code was written by someone who has been beating us nearly insensible about how > critical every single instruction is. So the code shown takes more instructions than > other alternatives, and he's been telling us that alternative implementations that take an > extra instuction or two are unacceptable implementations. So this code is inconsistent > with his previous concerns about performance. > > If there is irony here, it is the fact that he violates his own strongly-stated goals > about perfomance, I could not help but point out the inconsistency. My designs are the fastest possible designs. The implementations of these designs are not encoded to be the fastest possible encoding. I design the fastest code, and implement the most readable code. This was stated in my original specification but you missed it. I could say that missing such an important detail is pretty stupid, but I won't. By the way the single implementation choice of using a sentinel and gotos instead of a loop by itself produced 50% faster code. I am considering discarding this because requiring a NULL terminator seems a little too clumsy. > ***** >> >> >>> No, the CORRECT way to write such code is to either throw an exception (if >>> you are in C++, >>> which you clearly are) or return a value indicating the error (for >>> example, in C, an >> >> In this case, I'm for exception. >> Thanks to exception, you could use the precious function return value to >> actually return the resulting buffer (UTF8 string), instead of passing it as >> a reference to the function: > **** > I'd probably choose to throw an exception, where the exception information included the > offset into the input vector, a pointer to the input vector so the handler could decide > what to do, etc. > **** This is far too cumbersome in developmental code. Once the code has shown to be correct, then these sorts of enhancements can be made to derive production quality code. >> >> // Updated prototype: >> // - use 'const' correctness for utf32 >> // - return resulting utf8 >> // - may throw on error >> std::vector<uint8_t> toUTF8(const std::vector<uint32_t> & utf32); >> >> Note that thanks to the move semantics (i.e. the new "&&" thing of C++0x, >> available in VC10 a.k.a. VS2010), you don't pay for extra useless copies in >> returning potentially big objects. > **** > Yep. But this did not state it was a 2010-compliant version. Ultimately, there is a > philosophical inconsistency between his strongly-stated concerns about performance over > the last several months, and the actual implmentation presented here. Since he loves > picking nits with us, I felt it was only fair to return the favor. > > This does not change the fact that the printf is without a doubt a really awful interface. > joe NOT for developmental code. It requires less effort, thus less time to validate that the code is correct. Because of this alternatives to printf (for the purpose of validating code) tend to be inferior. > **** >> >> Giovanni >> >> > Joseph M. Newcomer [MVP] > email: newcomer(a)flounder.com > Web: http://www.flounder.com > MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Peter Olcott on 1 Jun 2010 09:52 On 6/1/2010 5:52 AM, Oliver Regenfelder wrote: > Hello, > > Leigh Johnston wrote: >> Also printf sucks, this is a C++ newsgroup not a C newsgroup. > > This is not even a general C++ newsgroup but an MFC one. So > strictly there is zero relevance of his posting to this > newsgroup. > > Best regards, > > Oliver So no one using MFC (such as I) would ever need to decode UTF-8?
From: Joseph M. Newcomer on 1 Jun 2010 10:03 See below... On Tue, 01 Jun 2010 08:51:05 -0500, Peter Olcott <NoSpam(a)OCR4Screen.com> wrote: >On 5/31/2010 9:16 PM, Joseph M. Newcomer wrote: >> See below... >> On Mon, 31 May 2010 20:16:53 +0200, "Giovanni Dicanio" >> <giovanniDOTdicanio(a)REMOVEMEgmail.com> wrote: >> >>> "Joseph M. Newcomer"<newcomer(a)flounder.com> wrote: >>> >>>>> UTF8.reserve(UTF32.size() * 4); // worst case >>>> **** >>>> Note that this will call malloc(), which will involve setting a lock, then >>>> searching for a >>>> block to allocate, then releasing the lock. Since you have been a fanatic >>>> about >>>> performance, why is it you put a very expensive operation like 'reserve' >>>> in your code? >>>> >>>> While it is perfectly reasonable, it seems inconsistent with your >>>> previously-stated goals. >>> >>> Joe: I'm not sure if you are ironic or something :) ... but I believe that >>> std::vector::reserve() with a proper capacity value, followed by several >>> push_back()s, is very efficient. >>> Sure, not as efficient as a static stack-allocated array, but very >>> efficient. >> **** >> But this code was written by someone who has been beating us nearly insensible about how >> critical every single instruction is. So the code shown takes more instructions than >> other alternatives, and he's been telling us that alternative implementations that take an >> extra instuction or two are unacceptable implementations. So this code is inconsistent >> with his previous concerns about performance. >> >> If there is irony here, it is the fact that he violates his own strongly-stated goals >> about perfomance, I could not help but point out the inconsistency. > >My designs are the fastest possible designs. **** By the use of reserve() and push_back(), the code you show is not the fastest possible *code*. I don't care about designs; designs are not executable! **** >The implementations of >these designs are not encoded to be the fastest possible encoding. I >design the fastest code, and implement the most readable code. This was >stated in my original specification but you missed it. I could say that >missing such an important detail is pretty stupid, but I won't. **** You really miss the point here. How could a *design* that requires an allocation be construed as something which could have an implementation that is the fastest possible code? Therefore, the design is not a design which lends itself to a fast encoding. If you had designed code to be fast, you would not have required the use of reserve() or push_back in the code! *You* were the one that insisted that designs include the final code, not me! A design that was intended to result in the fastest possible code would not include a requirement of a call to a storage allocator. So I can only assume that the design was, by your odd definition, not the "fastest possible" design. But I don't really care about designs; you showed an actual *code* artifact, and that is the ONLY thing that can be judged for its speed. Designs don't execute; code executes, and therefore the only thing you presented is a less-than-ideal implementation in terms of raw performance. **** > >By the way the single implementation choice of using a sentinel and >gotos instead of a loop by itself produced 50% faster code. I am >considering discarding this because requiring a NULL terminator seems a >little too clumsy. **** What is the problem with adding a NUL terminator (it is spelled NUL, with one L; that is the official designation of the 0 character in both ANSI and Unicode)? It is one push_back at the end. Remember, you can design so that if you return prematurely, the validity of the output buffer is undefined. joe **** > >> ***** >>> >>> >>>> No, the CORRECT way to write such code is to either throw an exception (if >>>> you are in C++, >>>> which you clearly are) or return a value indicating the error (for >>>> example, in C, an >>> >>> In this case, I'm for exception. >>> Thanks to exception, you could use the precious function return value to >>> actually return the resulting buffer (UTF8 string), instead of passing it as >>> a reference to the function: >> **** >> I'd probably choose to throw an exception, where the exception information included the >> offset into the input vector, a pointer to the input vector so the handler could decide >> what to do, etc. >> **** > >This is far too cumbersome in developmental code. Once the code has >shown to be correct, then these sorts of enhancements can be made to >derive production quality code. > >>> >>> // Updated prototype: >>> // - use 'const' correctness for utf32 >>> // - return resulting utf8 >>> // - may throw on error >>> std::vector<uint8_t> toUTF8(const std::vector<uint32_t> & utf32); >>> >>> Note that thanks to the move semantics (i.e. the new "&&" thing of C++0x, >>> available in VC10 a.k.a. VS2010), you don't pay for extra useless copies in >>> returning potentially big objects. >> **** >> Yep. But this did not state it was a 2010-compliant version. Ultimately, there is a >> philosophical inconsistency between his strongly-stated concerns about performance over >> the last several months, and the actual implmentation presented here. Since he loves >> picking nits with us, I felt it was only fair to return the favor. >> >> This does not change the fact that the printf is without a doubt a really awful interface. >> joe > >NOT for developmental code. It requires less effort, thus less time to >validate that the code is correct. Because of this alternatives to >printf (for the purpose of validating code) tend to be inferior. > >> **** >>> >>> Giovanni >>> >>> >> Joseph M. Newcomer [MVP] >> email: newcomer(a)flounder.com >> Web: http://www.flounder.com >> MVP Tips: http://www.flounder.com/mvp_tips.htm Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Joseph M. Newcomer on 1 Jun 2010 10:24 See below.., On Tue, 01 Jun 2010 08:38:24 -0500, Peter Olcott <NoSpam(a)OCR4Screen.com> wrote: >On 5/31/2010 9:10 PM, Joseph M. Newcomer wrote: >> See below... >> On Mon, 31 May 2010 12:17:00 -0500, Peter Olcott<NoSpam(a)OCR4Screen.com> wrote: >> >>> I am not that concerned with performance. The number one priority is >>> readability and maintainability. Performance is secondary to this. >> **** >> But in all your previous posts, you were declaring that the real value of a DFA was that >> it was the fastest possible implementation. Suddenly, performance no longer matters. > >No you merely screwed up yet again and misread what I said. You screw up >in this way quite often. This is your biggest single mistake. Your >second biggest mistake is gross over exaggeration. > >As one example: You said that nothing can be known about the efficiency >of a method without testing. I have empirically proven this statement >completely false. I accurately predicted exactly how much another UTF8 >decoder would be than my own code using the simple heuristic of >estimating the number of machine instructions in the execution path. 50% >more instructions did indeed result in 50% more time. **** Funny, I don't recall seeing the experimental results that demonstrated that. Results computed using QueryPerformanceCounter, showing the mean and standard deviation of a large number of experiments. Oh, and I don't recall you showing the code generated by that example, or giving potential instruction counts across a distribution of 1, 2, 3 and 4-byte encodings. Or even giving us data to indicate how many > 1-byte encodings you were handling. You are free to prove me wrong (you did it once, already, and I accept the data you collected), but without evidence, I am entitled to offer my opinion about the code, based on my considerable experience in writing, debugging, measuring, and generating machine code. Note: I have been an expert witness. When I am asked my "expert opinion" I am expected to give an opinion based upon my expertise. Such an opinion cannot be construed as libelous, because libel implies that I know the statement to be untrue. Look at case law on libel. But then again, I do have a certificate on Forensic Science and the Law from a local very important law school, and we had to study tort law as part of that. This means we had to understand what is meant by "libel" and what is meant by "expert". It would only be libel if I lied about what my experience had taught me, and said a statement inconsistent with my experience. My experience tells me that reserve() is a Really Bad Idea if your purpose is raw performance. My experience tells me that doing error reporting directly in a utility subroutine is poor design. Therefore, I am offering expert opinion based on my experience, and consistent with my experience. **** > >> >> And I still think the design is abysmal from a viewpoint of performance, usability, >> flexibility, and error reporting. Anyone who chooses to, in a general-purpose subroutine, >> pop up a MessageBox or do a printf, is simply clueless about how to design good software. >> **** > >I already warned you that such statements are libelous and that you >should cease and desist such statements. No one else here had indicated >that the design is less than good. **** Well, that might be because no one else spent 15 years doing performance measurement of programs. I look for little details, like the use of reserve() and push_back(). I was not aware that others had to agree with me to make a statement about code quality "non-libelous". In fact, you have no proof that using reserve() is zero cost, whereas I know how storage allocators work and know that its cost is always nonzero, and potentially very large (depending on how malloc is implemented), so making a statement to that effect is factual, not libelous. **** > >>>> From the bits viewpoint, it is probably correct (I didn't work out all lthe shifts and >>>> masks, figuring you probably got those right) but from a design and architecture >>>> viewpoint, it is truly awful code given the stated goals. Besides limiting it to console >>> >>> That statement could be libelous. Cease and desist any such commentary. >> **** >> What statement? That it is awful code given the stated goals? That can't be libelous. > >Do you really want to risk it? **** But the facts indicate otherwise. For my statement to be untrue, you would have to demonstrate that reserve() had zero cost, always. That push_back was faster than incrementing a target index and storing through it, under all conditions. Have you demonstrated this? ****. > >>> It is by no means truly awful code, anyone that knows code well would >>> know this. Prospective future employers might not know code well enough, >>> and believe what you said. >> **** >> Sorry, I would not hire someone who was dumb enough to put a printf in a piece of code >> that could be used in a Windows environment, or even in a console environment. > >Since no one else here thinks that my code is anything like abysmal that >shows that there is something else going on besides an objective >assessment of the quality of my code. **** OK, show the set of performance measurements, using a high-resolution timer, and showing how many experiments you ran, and the mean and standard deviation. Perferably for a variety of input length strings, say 100 characters, 1000 characters and 10,000 characters. This is how science is done. Telling me that your implementation is the fastest possible implementation when, in fact, it is potentially abysmal (do you know the potential cost of that reserve()? What if it takes a single page fault while searching the heap for a block of the right size?), is not "evidence". If you said "Here's an example of code" that's one thing. But promising us for weeks that you were going to implement something that was the fastest possible realization of an algorithm and then not actually showing that is inconsistent. Defending your "design" by a less-than-fastest-possible implementation is not "evidence". joe **** Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Joseph M. Newcomer on 1 Jun 2010 10:34
See below... On Mon, 31 May 2010 13:49:07 -0500, Peter Olcott <NoSpam(a)OCR4Screen.com> wrote: >On 5/31/2010 1:16 PM, Giovanni Dicanio wrote: >> "Joseph M. Newcomer" <newcomer(a)flounder.com> wrote: >> >>>> UTF8.reserve(UTF32.size() * 4); // worst case >>> **** >>> Note that this will call malloc(), which will involve setting a lock, >>> then searching for a >>> block to allocate, then releasing the lock. Since you have been a >>> fanatic about >>> performance, why is it you put a very expensive operation like >>> 'reserve' in your code? >>> >>> While it is perfectly reasonable, it seems inconsistent with your >>> previously-stated goals. >> >> Joe: I'm not sure if you are ironic or something :) ... but I believe >> that std::vector::reserve() with a proper capacity value, followed by >> several push_back()s, is very efficient. >> Sure, not as efficient as a static stack-allocated array, but very >> efficient. > >He needed to find some excuse to denigrate my code. He has had a >personal grudge against me for several months. I don't really know what >I said to offend him, but, it must have occurred sometime after he sung >very high praises about my patent a few months ago. *** I do not have a "personal grudge against you"; what I dislike are people who are pretentious, who make statements they can't back up, and present code that is inconsistent with their loudly-touted goals and try to make claims that it is the best possible code when it is not. I defended you against what I thought was an *unfair* accusation, that of being a Patent Troll. If there are unjust accusations, I will object. But when you batter us to insensibility about how critical performance is, and talk about presenting the "fastest possible design", then I am equally offended; designs cannot be executed and therefore cannot have speed. Code has measurable performance. And the code presented was bad code, for all the reasons I stated. It has nothing to do with a personal grudge; it has entirely to do with the fact that you state one thing, then present as evidence of your correctness something which contradicts your own statement. This is not consistent. Therefore, it is a target of opportunity to point out that you are not making sense. I also have to judge code for its correctness not just in the core algorithm, but in the overall implementation; utility code which uses printf or which even interacts with the user is not correct code, because it either will not work at all or will produce meaningless output to the user, and neither of these represent an acceptable design. If you make sense, I will defend you. If you prove me wrong with actual numbers, I will accept your numbers and agree that you are actually right. I did once before. But if you offer opnions on the performance of artficats that are measurable (code, not designs), without the data to back them, then you are not making sense, and you need to be told this. joe .. **** > >>> No, the CORRECT way to write such code is to either throw an exception >>> (if you are in C++, >>> which you clearly are) or return a value indicating the error (for >>> example, in C, an >> > >The "correct" way to handle an error when testing code for the first >time is to use a printf() statement, or other easy to use debugging >construct. When the code moves to production, then either of the other >two suggestions may be appropriate. > >> In this case, I'm for exception. >> Thanks to exception, you could use the precious function return value to >> actually return the resulting buffer (UTF8 string), instead of passing >> it as a reference to the function: >> >> // Updated prototype: >> // - use 'const' correctness for utf32 >> // - return resulting utf8 >> // - may throw on error >> std::vector<uint8_t> toUTF8(const std::vector<uint32_t> & utf32); > >For most compilers this requires making an extra copy. > >> >> Note that thanks to the move semantics (i.e. the new "&&" thing of >> C++0x, available in VC10 a.k.a. VS2010), you don't pay for extra useless >> copies in returning potentially big objects. >> >> Giovanni >> >> >> >Counting on this results in code that does not have the same performance >characteristics across multiple platforms. Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm |