@broadblues

I agree its not a bug in the library, but miss use of intention of open / close library,
of course it not horribly efficient to close and open the same library every few seconds,

the problem should be easy to prevent by simple changing one line to look some thing like this.

if (libcount < 0xFFFF ) libcount++;

of course you should probably trigger a grim or notification or some thing, but its not necessary.

A simple DebugPrintF() is useful as well.

Last edited by NutsAboutAmiga on 15-Apr-2011 at 08:55 PM.
Last edited by NutsAboutAmiga on 15-Apr-2011 at 08:51 PM.

_________________
http://lifeofliveforit.blogspot.no/
Facebook::LiveForIt Software for AmigaOS

@kas1e

_________________
http://lifeofliveforit.blogspot.no/
Facebook::LiveForIt Software for AmigaOS

@broadblues

Quote:

Sure I don't 'blame' him for a mistake (or even Fab for his "typo") it's the logic that then claims a a bug in the library that is misued, that sparked my post.

Yep. It is one of those WTF moments. It looks so innocent but drives developers to mental house

_________________
Amiga Developer
Amiga 500, Efika, Mac Mini and PowerBook

@NutsAboutAmiga

Quote:

agree its not a bug in the library, but miss use of intention of open / close library, of course it not horribly efficient to close and open the same library every few seconds, the problem should be easy to prevent by simple changing one line to look some thing like this. if (libcount < 0xFFFF ) libcount++; of course you should probably trigger a grim or notification or some thing, but its not necessary.

Now that would be a bug! In the admittedly unlikely event that the lib was legitamately opened more than "FFFF" times at the same time, then closed correctly there would be no way to correctly guage the "close count" leading to apremature expunge or something similar.

_________________
BroadBlues On Blues BroadBlues On Amiga Walker Broad

@broadblues

Not if the developer was notified about the problem.

It crashes as it is, so I does see how changing it for some more graceful is problem.

Last edited by NutsAboutAmiga on 15-Apr-2011 at 09:09 PM.

_________________
http://lifeofliveforit.blogspot.no/
Facebook::LiveForIt Software for AmigaOS

@NutsAboutAmiga

It's OpenLocale()/CloseLocale(), not OpenLibrary()/CloseLibrary(). And in this case, the value is cached by the locale library base, so the cost is very low anyway (especially compared to the calling code anyway).

@NutsAboutAmiga

You probably could crash it by closing locale once or twice too many. It is a shared resource and as a such always subject for memory trashing etc etc etc.

_________________
Amiga Developer
Amiga 500, Efika, Mac Mini and PowerBook

@itix

Quote:

In the admittedly unlikely event that the lib was legitimately opened more than "FFFF" times at the same time

in the event the lib_OpenCnt is legally opened more then 0xFFFF times on a 16bit counter, lib_OpenCnt will be 0, result: the library being Expunge'ed,

Is not better notify about the problem, and avoid crash there and then.

Last edited by NutsAboutAmiga on 15-Apr-2011 at 09:24 PM.
Last edited by NutsAboutAmiga on 15-Apr-2011 at 09:22 PM.
Last edited by NutsAboutAmiga on 15-Apr-2011 at 09:20 PM.
Last edited by NutsAboutAmiga on 15-Apr-2011 at 09:20 PM.

_________________
http://lifeofliveforit.blogspot.no/
Facebook::LiveForIt Software for AmigaOS

@Fab

Obs! I did not notice

_________________
http://lifeofliveforit.blogspot.no/
Facebook::LiveForIt Software for AmigaOS

@kas1e & Denil

Really REALLY GOOD NEWS!

Thank you! :)

_________________
Simone"Tuxedo"Monsignori, Perugia, ITALY.

@kas1e/deniil

It is foolish to open and close the resource every time you want to read a
value from the structure. It's that sort of unnecessary work that slows
down an application.

I would suggest that you change the code so that the resource is only opened
once (when the program starts), and is closed only once (when the program
exits). Write the address of the Locale structure in a global variable, along with all the library bases you have opened.
Then you could do away with the function altogether and replace it with a simple inline macro.

That would benefit all the target platforms.

_________________
cheers
tony

Hyperion Support Forum: http://forum.hyperion-entertainment.biz/index.php

@tonyw

Oh, that's a marvellous optimization you found there, really. It will surely improve the speed drastically... :)

Just for the record, that OpenLocale(NULL)/CloseLocale() pair takes 0.3 µs on my machine (measured with 1 million iterations to get a proper average), while the whole function calling this get_GMT_offset() function takes 1.3µs.

With this great idea of caching the default locale (or even the GMT for that matter), we go from 1.3µs to 1.0µs on average, for a function that is called quite often, sure, but not *that* often.

Now if we consider this function is called 260000 times in 34 minutes, that does about 127 calls per second. With this change, we save 381µs on 1000000 µs. Wouhou, we made OWB 0.03% faster. :)

Last edited by Fab on 16-Apr-2011 at 06:50 AM.
Last edited by Fab on 16-Apr-2011 at 06:47 AM.
Last edited by Fab on 16-Apr-2011 at 06:47 AM.
Last edited by Fab on 16-Apr-2011 at 06:45 AM.
Last edited by Fab on 16-Apr-2011 at 06:35 AM.

@Fab

So what you're saying is it's faster

@MickJT

Hahah well better faster than slower no ? even if only 0.0000001

_________________
BACK FOR THE FUTURE

http://www.betatesting.it/backforthefuture

Sam440ep Flex 800 Mhz 1 GB Ram + AmigaOS 4.1 Update 6
AmigaOne XE G3 800 Mhz - 640 MB Ram - Radeon 9200 SE + AmigaOS 4.1 Update 6

@Fab

Just one more reason to do it properly!

_________________
cheers
tony

Hyperion Support Forum: http://forum.hyperion-entertainment.biz/index.php

@Fab

No need to be cynical, you're usually much clever than this

_________________
PowerMac G5 2,7Ghz - 2GB - Radeon 9650 - MorphOS 3.14
AmigaONE X1000, 2GB, Sapphire Radeon HD 7700
FPGA Replay + DB 68060 at 85Mhz

@tonyw

Not just that. If you have a symetrical API (open, close) and you deliberatelly chose not to call close, because 'it doesn't do anything', you do a bit 'unfortunate' thing, because:
- You are programming against an interface, but you do assume what is the implementation. Even if Fab is a part of MOS dev team (not sure), this doesn't fiollow the abstraction the API imposes.
- because of that, the software is not future proof. The implementation might change, because the API allows it
- You make it less portable (i agree that portability wasn't the first goal of MUI OWB)

(excuse me, I'm not developing on amiga so take it with a pinch of salt, but I believe it is a general paradigm)

And, while I'm typing... Even if I'm not an amiga user these days, let me encourage you guys and express my opinion, namely, that you're doing a marvelous work, Roman and co. And Fab too - allowing to use his hard work is just great. Roman, I admire your very focus for adding the quality even into products done by others. Someone would call it nitpicking but I guess that's what this community needs! I'm following this portal since its start daily, but the fresh wind and activity (sharing knowledge and status) you bought in is just great. For me, it resembles the enthusiasm of the old times... ...just if I had a spare 1000 E to buy the system ....

Regards
Ed

Edit: hm..typos

Last edited by Treke on 16-Apr-2011 at 12:21 PM.

@Treke

I repeat... It *wasn't* on purpose. There are a couple other functions there using OpenLocale() and they're all matched with CloseLocale(), so I don't need any lesson on that, thank you. :)

My point here was just to explain why it didn't crash or had any ill effect even though the CloseLocale() was missing.

As for optimizations, I usually rather focus on the ones that give a sensible outcome (for instance making scroll with fixed elements 50 times faster, that kind of thing. :))

@Fab, kas1e, deniiil, Great work guys!!

_________________
AOS4 Betatester, Peg2, G4@1ghz, Radeon 9250 256mb, 1gb RAM.

http://www.asciiarena.com
http://www.uprough.net

Yes great work guys. When can we expect to see something to test. I know, I know ---- 2 more weeks.

TrevorD

_________________
No, I don't need no reason, I'm just breezin'