<div id="__MailbirdStyleContent" style="font-size: 10pt;font-family: Arial;color: #000000;text-align: left" dir="ltr">
Hi Eric --<div><br></div><div>Yes, I think it is worth improving Squeak's support for animated GIFs. Thanks!</div><div><br></div><div>> <span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">My concrete proposals, then, would be the following:</span></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">> 1. Remove AnimatedGIFReadWriter and replace GIFReadWriter with my updated version (along with helper classes)</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">> 2. Replace AnimatedImageMorph with my new BetterAnimatedImageMorph</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Well, for the sake of backwards compatibility, maybe you could do instead:</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">1*) Improve GIFReadWriter while ensuring that the obviously public protocol stays functional. Empty out AnimatedGIFReadWriter but leave it there.</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">2*) Improve AnimatedImageMorph while ensuring that the obviously public protocol stays functional. No new class BetterAnimatedImageMorph.</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Regarding the process:</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">- If single package: Inbox</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">- If multiple packages: Change set via squeak-dev</div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">:-)<br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Best,<br></div><div style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Marcel</div><div class="mb_sig"></div>
<blockquote class="history_container" type="cite" style="border-left-style: solid;border-width: 1px;margin-top: 20px;margin-left: 0px;padding-left: 10px;min-width: 500px">
<p style="color: #AAAAAA; margin-top: 10px;">Am 28.11.2021 21:10:36 schrieb Thiede, Christoph <christoph.thiede@student.hpi.uni-potsdam.de>:</p><div style="font-family:Arial,Helvetica,sans-serif">
<div id="divtagdefaultwrapper" style="font-size: 12pt;color: #000000;font-family: Calibri,Helvetica,sans-serif" dir="ltr">
<p>Hi Eric,</p>
<p><br>
</p>
<p>thanks for the reply. Of course, every test will be very valuable!</p>
<p><br>
</p>
<p>Regarding efficiency and optimization: There are other people who might be able to give you detailed information, but maybe I can add a few basic notes:</p>
<p><br>
</p>
<p>> <span>I am very likely using inappropriate classes or data structures for some of the decoding.</span></p>
<p><br>
</p>
<p>Hypothetically any subclass of RawBitsArray (maybe even create a new one?) could be your friend (this hierarchy has been reworked recently, so maybe it takes some time to find the right structure.)</p>
<p><br>
</p>
<p>> <span style="font-size: 12pt">Worse case scenario: we offload the encoding/decoding of LZW data to a plugin.</span></p>
<div><br>
</div>
<div>Or use FFI instead. But whenever possible, I would myself prefer a transparent implementation inside the image.</div>
<div><br>
</div>
<div>> <span style="font-size: 12pt">That said, are there other packages available for profiling etc, or is MessageTally class >> spyOn: the bread and butter for that kind of thing?</span>
<div><br>
</div>
<div>There is the InteractiveProfilingTool <<a href="https://github.com/hpi-swa-teaching/InteractiveProfilingTool" class="OWAAutoLink" id="LPlnk724670" previewremoved="true">https://github.com/hpi-swa-teaching/InteractiveProfilingTool</a>> which aggregates
the recorded data a bit more conveniently, or you could evaluate the following: <span style="font-size: 12pt">[self doSomeOperation] timeProfile. But it's not a completely new approach, just some slight refinements.</span></div>
<div>Apart from that, we have SpaceTally for measuring object sizes (see the class comment), but I have not yet used it myself.</div>
<div>If you want to analyze the usage of different VM primitives and modules, you could try out my new
<span>BenchmarkSimulator from SimulationStudio <<a href="https://github.com/LinqLover/SimulationStudio" class="OWAAutoLink" id="LPlnk78614" previewremoved="true">https://github.com/LinqLover/SimulationStudio</a>> (experimental), but it is itself very slow.</span></div>
<div><br>
</div>
<div>> <span style="font-size: 12pt">On a related note, I did notice that during GIF frame composition there was what seemed to me a lot of time spent on symbol/string comparison at one point. (`9.5% {192ms} ByteSymbol(Symbol)>>=`). I don't quite understand
why that should be the case.</span>
<div><br>
</div>
<div>For symbols, you can use #== instead which is compiled as an inlined message send (-> faster).</div>
<div><br>
</div>
<div>Have a nice weekend!</div>
<div><br>
</div>
<div>Best,</div>
<div>Christoph</div>
</div>
</div>
<p></p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><span style="font-family: Calibri, sans-serif;color: #000000"><b>Von:</b> Squeak-dev <squeak-dev-bounces@lists.squeakfoundation.org> im Auftrag von Eric Gade <eric.gade@gmail.com><br>
<b>Gesendet:</b> Sonntag, 28. November 2021 17:31:17<br>
<b>An:</b> The general-purpose Squeak developers list<br>
<b>Betreff:</b> Re: [squeak-dev] [Discussion] GIF and Animated GIF Refactoring</span>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>Hi Christoph,</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Sat, Nov 27, 2021 at 2:12 PM Thiede, Christoph <<a href="mailto:Christoph.Thiede@student.hpi.uni-potsdam.de">Christoph.Thiede@student.hpi.uni-potsdam.de</a>> wrote:</div>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex;border-left-width: 1px;border-left-style: solid;border-left-color: rgb(204,204,204);padding-left: 1ex;min-width: 500px">
<div dir="ltr">
<div id="gmail-m_-3387757843769282171divtagdefaultwrapper" style="font-size: 12pt;color: rgb(0,0,0);font-family: Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols" dir="ltr">
<p></p>
<ul style="margin-bottom:0px;margin-top:0px">
<li><span>compatibility checks - are there any breaking changes in terms of behavior, or has the public protocol been changed? Maybe it would be worth improving compatibility, adding a few "deprecated" methods, autc. For instance, probably we should re-add
an empty subclass <span>AnimatedGIFReadWriter in the 60Deprecated/61Deprecated package to not break existing code that relies on it. Another example, #<span>openGIFInWindow:/#fromStream: are not understood by BetterAnimatedImageMorph class.</span></span></span></li></ul>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>I agree. None of this replacement should happen without preserving that polymorphism, at least for the time being. I should probably write a few tests along the way (I have only a handful from previous ports)</div>
<div> </div>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex;border-left-width: 1px;border-left-style: solid;border-left-color: rgb(204,204,204);padding-left: 1ex;min-width: 500px">
<div dir="ltr">
<div id="gmail-m_-3387757843769282171divtagdefaultwrapper" style="font-size: 12pt;color: rgb(0,0,0);font-family: Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols" dir="ltr">
<ul style="margin-bottom:0px;margin-top:0px">
<li><span>code review - not sure whether there is anyone who would like to have a detailed look at your patch before it is integrated. But as you say it is "tested by the crowd" on two different Smalltalk systems, my personal opinion would be that this should
not be a reason to block great contributions.</span></li></ul>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>I believe I have some tests in ports to the other Smalltalks but I'll probably have to refactor them. In particular, there is and should be a test for reading a GIF, writing it out, reading it back in, and having the correct pixels at expected locations. </div>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex;border-left-width: 1px;border-left-style: solid;border-left-color: rgb(204,204,204);padding-left: 1ex;min-width: 500px">
<div dir="ltr">
<div id="gmail-m_-3387757843769282171divtagdefaultwrapper" style="font-size: 12pt;color: rgb(0,0,0);font-family: Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols" dir="ltr">
PS: Have you or someone else already tried to optimize the new GIFReadWriter for performance? I am having a 36 MB large
<a href="https://github.com/LinqLover/TelegramSmalltalkBot/blob/master/img/screencast.gif" target="_blank">
GIF</a> here which takes eternities (> 10 minutes) to file in on my machine. With a 9 MB small GIF, it still takes a few minutes. And the RAM consumption of my image is noticeably high. But after that, I can play the GIF with ca. 25 FPS. However, the old implementation
was not faster either.</div>
</div>
</blockquote>
<div><br>
</div>
<div>It's actually worse than you think -- I cannot spy on the process of parsing your large GIF file without my Squeak process eventually killing itself, probably due to memory consumption. As it stands, I don't know the exact reason the memory consumption
is so high. There *must* be some places for optimization in the decoding. When I rewrote the LZW encoding/decoding portions of this, my immediate goal was to understand what was going on -- therefore being verbose and using more objects. I am very likely using
inappropriate classes or data structures for some of the decoding. Worse case scenario: we offload the encoding/decoding of LZW data to a plugin.</div>
<div> </div>
<div>There is also the matter of working with the Bitmap and Form objects at a low level, which I must admit I still don't fully grasp. I could be missing something there. <br>
</div>
<div> </div>
<div>That said, are there other packages available for profiling etc, or is MessageTally class >> spyOn: the bread and butter for that kind of thing?</div>
<div> </div>
<div>On a related note, I did notice that during GIF frame composition there was what seemed to me a lot of time spent on symbol/string comparison at one point. (`9.5% {192ms} ByteSymbol(Symbol)>>=`). I don't quite understand why that should be the case.</div>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">
<div>Eric</div>
</div>
</div>
</div>
</div>
</div></blockquote></div>