[Seaside] Seaside2.7a1-avi.10

[Philippe Marschall]

2006/9/22, Avi Bryant <avi.bryant at gmail.com>:
> Note: this message is only relevant for people using Seaside on
> Squeak.  I think.
>
> I've just committed a version to the 2.7a branch which includes a
> couple of changes we use internally for Dabble DB.  The main change I
> just made recently, and although it seems fine, it's hackish enough
> that I'd be curious to have others take a look at it.  It takes a bit
> of explanation.
>
> The background is this: we had some users complaining that if they
> opened up a number of tabs in their browser, sometimes when they went
> back to an old tab the links they clicked on would have no effect.
> This is, of course, because we expire old continuations, and their
> old tabs were falling out of the LRU cache.
>
> So I tried bumping up (way up, to like 500) the number of
> continuations kept in the cache.  Not that surprisingly, I saw a
> massive increase (cripplingly so) in the size of the Seaside
> sessions.  The interesting thing was what a space analysis of these
> huge images showed.  The space wasn't, largely, being used by stored
> method contexts or snapshotted state.  It was almost all instances of
> String, and some random sampling of these showed that almost all of
> them were from CSS class or id attributes.  Huh?
>
> What was happening was this:  I would have a render method that would
> construct a CSS class or ID, usually by combining some string literal
> with some number.  Then I would pass that class or ID into another
> render method.  Minimally, it might look like this:
>
> renderCellForColumn: aNumber on: html
>     |class|
>     class := 'tableColumn', (aNumber \\ 2 + 1) asString.
>     self renderCellWithClass: class on: html
>
> Then that other render method would generate a callback, like this:
>
> renderCellWithClass: aString on: html
>    html tableData class: aString; with:
>       [html anchor callback: [self doStuff]; text: 'Do stuff']
>
> Because aString was constructed, it was a new instance every time.
> And because it was in the home context for the callback block (even
> though it wasn't used from the block), it was held onto by the
> callback, which is held onto for (in my case) quite a long time.  And
> when you have tables with thousands of cells, each with one of these
> CSS classes, it builds up.
>
> So, ok, one lesson here is to be more careful about constructing
> strings (I could have 'tableColumn1' and 'tableColumn2' literals and
> an if statement), or to intern them.  But I was curious if I could
> solve the problem more generally than that.  After all, the block
> doesn't *need* to be holding onto aString.
>
> So I added a #tempVarRefs method to BlockContext.  This scans the
> bytecodes for the block and returns the indices of any temps that the
> block uses.  I think added a variation of #fixTemps named
> #fixCallbackTemps which, after copying the home context, goes through
> the temps and nils out any that don't appear in #tempVarRefs.
>
> I then changed every reference to #fixTemps within Seaside to
> #fixCallbackTemps.  The upshot is that all stored callbacks in
> Seaside should avoid holding onto any temps that they don't actually
> use.
>
> I'm hoping this will lead to a noticeable reduction in memory
> footprint in the general case, but I haven't done any tests yet.  I
> also haven't done any thorough testing of #tempVarRefs to make sure
> that #fixCallbackTemps does, in fact, leave the semantics of the
> block unchanged compared to #fixTemps.  I would love it if someone
> had the time to review the code and run some such tests.
>
> Meanwhile, we're using it in production with no ill effects reported
> so far...

It's great to have you back doing Seaside development.

I can not help but think if we had a VM that ....

Philippe