http://d.puremagic.com/issues/show_bug.cgi?id=5813
--- Comment #4 from Rob Jacques <sandf...@jhu.edu> 2011-06-09 12:40:52 PDT --- (In reply to comment #3) > I think this is a good idea to do, but the code is somewhat obfuscated. For > example, you are returning a value from a void function (extend), and doing > some weird shortcuts (like !false*2). There are also some style differences > from the current D style guidelines. These will have to be fixed before it's > put into phobos. The obfuscation generally comes from a desire to minimize multi-lines to one-liners. But if you think these are too compressed, I'll change them to something cleaner. Regarding D style guidelines, there has been so much discussion over them, that I don't know if/where there current guidelines exist/are. And the existing Phobos codebase is a mash-up of different styles. I mean, even ddoc comments vary between /++ and /**. Is there anything in particular you'd recommend I'd change? > One technical point -- the limit of multiples of page sizes when requesting > more than one page is already done by the GC. That is, If you request more > than 1/2 page, you will get a multiple of pages. I think the code will > perform > exactly as well if the multiple-of-page limitation is removed from your code, > and this then does not make any assumptions about how big a page size is (if > it > ever changes). Thank you. I'll remove that line. > Do you want to do a pull request and have the code reviewed? I think it's > definitely worth changing Appender to do this. Yes, but I haven't gotten around to learning/installing Git on Windows yet. > One further thing -- I planned on making two versions of Appender: one that is > a reference type, and one that is not. The version that is not should allow > filling an existing an array without allocating anything. The current > Appender > (the one in phobos now) always allocates an pImpl struct, even if the array > never needs to be extended. > > The tradeoff is that the non-reference appender is not returnable, and you > can't make copies of it (i.e. this(this) is @disabled). > > Is this possible with your code? If so, and it's easy enough, can you create > a > version that does this too? The simple solution would be to emplace the first data node inside the appender struct. But I'm not sure the benefit of such a solution: appender always checks/extends-to the capacity of an array, which involves taking the GC lock, etc. So since the taking lock can't be averted, avoiding an extra 16-byte allocation isn't much worse. But I can always implement it using a template parameter + static ifs, so it shouldn't be hard to try it out. Hmm... I could also fold in RefAppender that way. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------