http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660Nitin, No, it doesn't do thathttp://ayende.com/147457/stupid-smart-code#comment52http://ayende.com/147457/stupid-smart-code#comment52Thu, 22 Dec 2011 11:09:30 GMTInternally before counting the number of characters?http://ayende.com/147457/stupid-smart-code#comment51http://ayende.com/147457/stupid-smart-code#comment51Thu, 22 Dec 2011 10:47:53 GMTNitin, GetBytesCount does NOT create an arrayhttp://ayende.com/147457/stupid-smart-code#comment50http://ayende.com/147457/stupid-smart-code#comment50Thu, 22 Dec 2011 10:33:11 GMTvar byteCount = Encoding.UTF8.GetByteCount(data); GetByteCount creates one more array of characters? http://ayende.com/147457/stupid-smart-code#comment49http://ayende.com/147457/stupid-smart-code#comment49Thu, 22 Dec 2011 10:30:59 GMTreq.GetRequestStream() returns an instance of internal ConnectStream class, whose Flush() method for some reason does exactly nothing...http://ayende.com/147457/stupid-smart-code#comment47http://ayende.com/147457/stupid-smart-code#comment47Wed, 21 Dec 2011 18:57:15 GMTTobi, That is why I moved to stream writer there in the first place. And the flushing merely ensures that the data goes out. Without it, you are reliant on the Dispose calling Flush, and there are good reasons not of call it, sometimes. For example, you might call Dispose on the writer, which will dispose the stream, but you don't want that. One example why you want that.http://ayende.com/147457/stupid-smart-code#comment46http://ayende.com/147457/stupid-smart-code#comment46Wed, 21 Dec 2011 13:11:28 GMTAyende, but data consistency is not one. Memory usage should be controlled by configuring the buffer size. You cannot release memory by flushing. The OS will flush to disk but still have the data cached. Network latency: true. Rarely needed and reduces throughput.http://ayende.com/147457/stupid-smart-code#comment45http://ayende.com/147457/stupid-smart-code#comment45Wed, 21 Dec 2011 13:07:53 GMTTobi, No, it isn't. There are a lot of reasons to flush. Reducing memory usage, for example, ensuring lower latency in network scenarios, for another. http://ayende.com/147457/stupid-smart-code#comment44http://ayende.com/147457/stupid-smart-code#comment44Wed, 21 Dec 2011 11:52:16 GMTIt is almost always wrong to flush. The only reason I can come up with is to harden data on disk immediately for durability semantics. Flush = fsync. It is _always_ slower and does not change the data written at all.http://ayende.com/147457/stupid-smart-code#comment43http://ayende.com/147457/stupid-smart-code#comment43Wed, 21 Dec 2011 11:37:28 GMThmm... don't know; this seems a little like "select ain't broken" but... clearly i'm not the smartest guy in the room but i would think that you would want to flush the internal state of the encoder of course i have been bitten by this type of post in the past to where it could be that there was no check for the "data" parameter being null or emptyhttp://ayende.com/147457/stupid-smart-code#comment41http://ayende.com/147457/stupid-smart-code#comment41Wed, 21 Dec 2011 07:50:42 GMTMy guess is that since you instantiate a new "bytes" var in every loop it actually ends up holding everything in memory twice anyways(at least until the GC kicks in). http://ayende.com/147457/stupid-smart-code#comment40http://ayende.com/147457/stupid-smart-code#comment40Wed, 21 Dec 2011 00:56:15 GMTdata.CopyTo(...) copies the value and doesn't reference the original char?http://ayende.com/147457/stupid-smart-code#comment39http://ayende.com/147457/stupid-smart-code#comment39Tue, 20 Dec 2011 22:35:04 GMTJarret + 0x1000! Seeing as these are strings being passed back on a request I cannot see them being HUGE. (many MBs) I'd expect if anything was a memory drain it would be from the server serving many requests in which case the complex code would solve nothing. Temporarily allocating a few extra K for byte arrays to maintain readable code is certainly worth it.http://ayende.com/147457/stupid-smart-code#comment38http://ayende.com/147457/stupid-smart-code#comment38Tue, 20 Dec 2011 21:36:00 GMTUse Stream.Copyhttp://ayende.com/147457/stupid-smart-code#comment37http://ayende.com/147457/stupid-smart-code#comment37Tue, 20 Dec 2011 20:05:58 GMTTell us what you see Ayendehttp://ayende.com/147457/stupid-smart-code#comment36http://ayende.com/147457/stupid-smart-code#comment36Tue, 20 Dec 2011 20:00:42 GMTI'm not sure I understand. In first solution you say memory is allocated twice. One for the string (parameter?) and for the buffer. In second solution memory is allocated for string and then for buffer but multiple times. var buffer... var charBuffer... and in a while loop var bytes So what you gain is that when data is very big you don't end up allocating a lot of memory on LOH. Instead you do a lot of small allocations if data is big. Don't know what you mean by being stupid. I don't like the code before and after modification if data is very big. If it is not then why bother changing original code? http://ayende.com/147457/stupid-smart-code#comment35http://ayende.com/147457/stupid-smart-code#comment35Tue, 20 Dec 2011 19:00:07 GMTHow about flushing inside the loop if you want the data streamed? Let's call it lots of "courtesy flushes"http://ayende.com/147457/stupid-smart-code#comment34http://ayende.com/147457/stupid-smart-code#comment34Tue, 20 Dec 2011 18:33:50 GMTAs other pointed out I'm on the side that the "I'm stupid" epiphany was due to realizing that you just reimplemented logic that is already implemented by StreamWriter.http://ayende.com/147457/stupid-smart-code#comment33http://ayende.com/147457/stupid-smart-code#comment33Tue, 20 Dec 2011 18:32:59 GMTI believe the problem is two-fold: 1) GetByteCount(string s) converts the string into char[s.Length] - so nothing has been gained there 2) worse still, GetByteCount has to encode the characters to produce its result, then the characters are encoded a 2nd time to write to the streamhttp://ayende.com/147457/stupid-smart-code#comment32http://ayende.com/147457/stupid-smart-code#comment32Tue, 20 Dec 2011 16:54:15 GMT@Frank Quednau - the first version is like it is due to the need to set ContentLength before writing to the streamhttp://ayende.com/147457/stupid-smart-code#comment31http://ayende.com/147457/stupid-smart-code#comment31Tue, 20 Dec 2011 16:51:25 GMTWhat's the gain in performance compared to the extra complexity of the code?http://ayende.com/147457/stupid-smart-code#comment30http://ayende.com/147457/stupid-smart-code#comment30Tue, 20 Dec 2011 16:42:22 GMTJarrett indeed is the first one who mentions the first thing that popped into my mind when reading this: "why change something which intent initially was perfectly obvious into something that you need to read multiple times and is a lot harder to understand for a micro-optimization that may not even necessarily be an issue?" Doesn't seem like it's worth the extra complexity it introduces at a cost of read- and maintainability. http://ayende.com/147457/stupid-smart-code#comment29http://ayende.com/147457/stupid-smart-code#comment29Tue, 20 Dec 2011 15:54:10 GMTConfused by the first version in the first place. The simplest abstraction to write a string to a stream is a streamwriter, is it not? using (var w = new StreamWriter(r.GetRequestStream(), Encoding.UTF8)) sw.Write(data);http://ayende.com/147457/stupid-smart-code#comment28http://ayende.com/147457/stupid-smart-code#comment28Tue, 20 Dec 2011 15:35:11 GMTI'm guessing this code is stupid because you probably don't have any profiling metrics that show it actually needs optimization in the first place. So, rather than fixing true bottlenecks or implementing new features, you have needlessly invested a lot of time and introduced much more complexity.http://ayende.com/147457/stupid-smart-code#comment27http://ayende.com/147457/stupid-smart-code#comment27Tue, 20 Dec 2011 15:09:15 GMTJarrett is the only one who's got it so far. Do you really need the extra code complexity and lack of maintainability for that performance gain? Do you have evidence that the extra memory allocation was really hurting the system?http://ayende.com/147457/stupid-smart-code#comment26http://ayende.com/147457/stupid-smart-code#comment26Tue, 20 Dec 2011 14:44:54 GMTIf the method is not going to write big chunk of data, then you have increased the complexity instead of keeping it simplehttp://ayende.com/147457/stupid-smart-code#comment25http://ayende.com/147457/stupid-smart-code#comment25Tue, 20 Dec 2011 14:26:21 GMTYou can use the lambda expression and execute it in one line: Var reader = new StringReader(data); reader.CopyTo(req.GetRequestStream();http://ayende.com/147457/stupid-smart-code#comment24http://ayende.com/147457/stupid-smart-code#comment24Tue, 20 Dec 2011 12:45:01 GMT@tobi, missed that one, although I only looked inside the loop @JD, bytes is an int, not much GC-ing there :)http://ayende.com/147457/stupid-smart-code#comment23http://ayende.com/147457/stupid-smart-code#comment23Tue, 20 Dec 2011 11:56:05 GMT@JD - 'bytes' is actually an int. The joys of var and ambiguous variable names!http://ayende.com/147457/stupid-smart-code#comment22http://ayende.com/147457/stupid-smart-code#comment22Tue, 20 Dec 2011 11:56:00 GMT@tobi - my understanding is that the optimisation was intended to avoid allocating memory for the (potentially large) request body twice, not to minimise encoding time. http://ayende.com/147457/stupid-smart-code#comment21http://ayende.com/147457/stupid-smart-code#comment21Tue, 20 Dec 2011 11:35:04 GMT