Plugins / Memory Usage / Performance - please read.

[Dan Sully]

So during my insomnia last night, and tracking down memory leaks yesterday, I
came up with the following (this is mostly a brain dump), and would like feedback.

Memory savings:

Only load Slim::Formats::* modules on demand - Saves about 5M for the common case.

Default importer plugins to off (but available in settings) - most users won't need them.

Turn LineX, Snow, etc off (but available in settings) by default - most users won't need them.

Strings:

at Runtime, create tied DB_File's per language, so strings.txt
doesn't need to be in memory all the time.

Importers:

Check audiodir against the path - only do readTags if needed.

iTunes - have Playlist only selection

Thoughts? Most of these (excecpt for Strings) are doable, and almost required for 6.0

-D
--
<iNoah> all your base class are belong to us

[kdf]

Quoting Dan Sully <dan (AT) slimdevices (DOT) com>:

> So during my insomnia last night, and tracking down memory leaks yesterday, I
> came up with the following (this is mostly a brain dump), and would like
> feedback.
>
> Memory savings:
>
> Only load Slim::Formats::* modules on demand - Saves about 5M for the common
> case.

Sounds great. We use them rarely enough and then really dont need them after
scanning.

>
> Default importer plugins to off (but available in settings) - most users
> won't need them.

the importers should basically disable themselves if the conditions arent right.
Moodlogic will disable if the OS is not windows, or the COM object isnt'
found, etc. Is this not enough?

> Turn LineX, Snow, etc off (but available in settings) by default - most
> users won't need them.

If its a plugin, I could probably be convinced that they should all be disabled
by default. If not, then maybe they shouldnt' be plugins

>
> Strings:
>
> at Runtime, create tied DB_File's per language, so strings.txt
> doesn't need to be in memory all the time.
>
> Importers:
>
> Check audiodir against the path - only do readTags if needed.

Brilliant idea!!

> iTunes - have Playlist only selection

sounded like a good idea from the start. Let me know if you want me to deal
with that one (given that you are likely well overloaded already)

-kdf

[Dan Sully]

* kdf shaped the electrons to say...

>> Only load Slim::Formats::* modules on demand - Saves about 5M for the common
>> case.
>
>Sounds great. We use them rarely enough and then really dont need them after scanning.

That's correct.

>> Default importer plugins to off (but available in settings) - most users won't need them.
>
>the importers should basically disable themselves if the conditions arent right.
> Moodlogic will disable if the OS is not windows, or the COM object isnt'
>found, etc. Is this not enough?

No, it's not - the plugins (and memory) is already used at that point.

I just checked in a change to not load MoodLogic at all if we're not on Windows.

Dean wants me to hold off on the others until post 6.0. iTunes should "just
work" if it's installed, and the user not have to do anything.

>> Turn LineX, Snow, etc off (but available in settings) by default - most
>> users won't need them.
>
>If its a plugin, I could probably be convinced that they should all be disabled
>by default. If not, then maybe they shouldnt' be plugins

I just removed LineX and Snow, as they don't even work on all our devices. ~600k savings.

They'll be offered on the plugins download page.

>> Importers:
>>
>> Check audiodir against the path - only do readTags if needed.
>
>Brilliant idea!!

In talking with Dean, and looking at code, readTags is a no-op in the case of
having MusicFolderScan, and a Importer. Whichever one gets to a track first
will import some information to the DB, at which point the next one gets
there, calls updateOrCreate(), even with readTags set to true, will see it
already has the track in the DB, and only update it, not readTags again.

>> iTunes - have Playlist only selection
>
>sounded like a good idea from the start. Let me know if you want me to deal
>with that one (given that you are likely well overloaded already)

We're going to go with this as an "Advanced" iTunes option - not on the main page, and post 6.0

-D
--
"They that can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety." - Benjamin Franklin

[kdf]

Quoting Dan Sully <dan (AT) slimdevices (DOT) com>:


>
> No, it's not - the plugins (and memory) is already used at that point.
>
> I just checked in a change to not load MoodLogic at all if we're not on
> Windows.
>
I saw that. Its a shame to be adding something hardcoded for a plugin. But, I
can understand that unloading post check or from inside the plugin really isn't
an option.

> Dean wants me to hold off on the others until post 6.0. iTunes should "just
> work" if it's installed, and the user not have to do anything.

makes sense. we've had that as a standard for itunes for some time. Its too bad
we can't only partly load a module, check if all valid conditions exist, then
load the rest or save memory by not loading the rest.

> >
> >Brilliant idea!!
>
> In talking with Dean, and looking at code, readTags is a no-op in the case of
> having MusicFolderScan, and a Importer. Whichever one gets to a track first
> will import some information to the DB, at which point the next one gets
> there, calls updateOrCreate(), even with readTags set to true, will see it
> already has the track in the DB, and only update it, not readTags again.

well, I've wondered. In the case of the rescal log from Craig James, the debug
lines seem to indicate data being updated from d_info. I haven't had time to
look it up, so they might be spitting out regardless of no-op, but I did wonder
if those were indicating a new run. I guess it is really just a confirmation
of the updates.

>
> >> iTunes - have Playlist only selection
> >
> >sounded like a good idea from the start. Let me know if you want me to deal
> >with that one (given that you are likely well overloaded already)
>
> We're going to go with this as an "Advanced" iTunes option - not on the main
> page, and post 6.0

sounds good to me

[Dan Sully]

* kdf shaped the electrons to say...

>> Dean wants me to hold off on the others until post 6.0. iTunes should "just
>> work" if it's installed, and the user not have to do anything.
>
>makes sense. we've had that as a standard for itunes for some time. Its too bad
>we can't only partly load a module, check if all valid conditions exist, then
>load the rest or save memory by not loading the rest.

There is the Autoloader mechanism, but I don't want to deal with that right now.

>> In talking with Dean, and looking at code, readTags is a no-op in the case of
>> having MusicFolderScan, and a Importer. Whichever one gets to a track first
>> will import some information to the DB, at which point the next one gets
>> there, calls updateOrCreate(), even with readTags set to true, will see it
>> already has the track in the DB, and only update it, not readTags again.
>
>well, I've wondered. In the case of the rescal log from Craig James, the debug
>lines seem to indicate data being updated from d_info. I haven't had time to
>look it up, so they might be spitting out regardless of no-op, but I did wonder
>if those were indicating a new run. I guess it is really just a confirmation
>of the updates.

Yeah - it will update the info, but it's not reading tags again.

-D
--
Adobe Photoshop - When you want the truth. Real bad.

[mherger]

[..]
> Strings:
>
> at Runtime, create tied DB_File's per language, so strings.txt
> doesn't need to be in memory all the time.

Another idea: most of the users won't need more than 2 languages (their
preferred + english as fallback). Today we have 12 of them and all stay in
memory. Couldn't we read the strings dynamically on language change? Maybe
even only one language and do the fallback check when loading the strings.
Again: most users won't change the language once they've selected their
language.

I would have to rewrite my translator/stringeditor plugin, though...

> Thoughts? Most of these (excecpt for Strings) are doable, and almost
> required for 6.0

Somebody told me "only bug fixes, no new features before the release" a
few days back ;-)

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

[kdf]

Quoting Michael Herger <slim (AT) herger (DOT) net>:


> > Thoughts? Most of these (excecpt for Strings) are doable, and almost
> > required for 6.0
>
> Somebody told me "only bug fixes, no new features before the release" a
> few days back ;-)

I think a good argument could me made to qualify "unnecessary memory usage, and
lagging performance" as a bug.

-kdf

[Dan Sully]

* kdf shaped the electrons to say...

>> > Thoughts? Most of these (excecpt for Strings) are doable, and almost
>> > required for 6.0
>>
>> Somebody told me "only bug fixes, no new features before the release" a
>> few days back ;-)
>
>I think a good argument could me made to qualify "unnecessary memory usage, and
>lagging performance" as a bug.

That is correct - these are issues which hamper the performance and usability of SlimServer.

After the 6.0 release (the 25th or so), we'll make a 6.1 branch, which new features can go into.

-D
--
"They that can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety." - Benjamin Franklin

[mherger]

On Thu, 17 Mar 2005 13:54:41 -0800, Dan Sully <dan (AT) slimdevices (DOT) com> wrote:

> * kdf shaped the electrons to say...
>
>>> > Thoughts? Most of these (excecpt for Strings) are doable, and almost
>>> > required for 6.0
>>>
>>> Somebody told me "only bug fixes, no new features before the release" a
>>> few days back ;-)
>>
>> I think a good argument could me made to qualify "unnecessary memory
>> usage, and
>> lagging performance" as a bug.
>
> That is correct - these are issues which hamper the performance and
> usability of SlimServer.

Ok, you're right, of course. But what about my main message about the
strings?

> Another idea: most of the users won't need more than 2 languages (their
> preferred + english as fallback). Today we have 12 of them and all stay
> in memory. Couldn't we read the strings dynamically on language change?
> Maybe even only one language and do the fallback check when loading the
> strings. Again: most users won't change the language once they've
> selected their language.

I did a quick hack to only include DE and EN - memory consumption for the
%strings hash dropped from about 1MB to 400kb. For english only it's even
down to 290kb.

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

[kdf]

Quoting Michael Herger <slim (AT) herger (DOT) net>:

> On Thu, 17 Mar 2005 13:54:41 -0800, Dan Sully <dan (AT) slimdevices (DOT) com> wrote:
>
> > * kdf shaped the electrons to say...
> >
> >>> > Thoughts? Most of these (excecpt for Strings) are doable, and almost
> >>> > required for 6.0
> >>>
> >>> Somebody told me "only bug fixes, no new features before the release" a
> >>> few days back ;-)
> >>
> >> I think a good argument could me made to qualify "unnecessary memory
> >> usage, and
> >> lagging performance" as a bug.
> >
> > That is correct - these are issues which hamper the performance and
> > usability of SlimServer.
>
> Ok, you're right, of course. But what about my main message about the
> strings?
>
> > Another idea: most of the users won't need more than 2 languages (their
> > preferred + english as fallback). Today we have 12 of them and all stay
> > in memory. Couldn't we read the strings dynamically on language change?
> > Maybe even only one language and do the fallback check when loading the
> > strings. Again: most users won't change the language once they've
> > selected their language.
>
> I did a quick hack to only include DE and EN - memory consumption for the
> %strings hash dropped from about 1MB to 400kb. For english only it's even
> down to 290kb.
>

sounds promising. assuming a final release of 6.0 about 7-10 days from now, can
you turn that into a working patch?

I'd certainly call it a bug fix, myself!
-kdf