Show Posts
This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.
Messages - Nao
4336
Features / Re: New revs - Public comments
« on October 8th, 2012, 06:13 PM »So we go through the few hundred variables, testing for { and unserialize every page load. Sounds nice and efficient.
How many rows is that actually applied to? You need to consider around 400-500 rows for a typical install.
But more than that, I guarantee it WILL introduce issues because it's not just the loading code that needs to be fixed, all of the saving code also all needs to be fixed.
Topic Solved doesn't seem to worry about it too much, seeing how certain types of setting can themselves be arrays, which are already serialised, such as multi-select or boards settings.
Do it if you really want but I'm really not keen on it because I'm not convinced it actually saves anything. The problem isn't loading, it's saving. That, and all the fun code related to editing settings in the admin panel, which was crappy enough to write the first time.
4337
Features / Re: New revs - Public comments
« on October 8th, 2012, 06:10 PM »
Another test...
- Disabled caching for $settings
- Added unserialize directly to the $settings[$row[0]] = $row[1] line
- Result is about 40% slower, i.e. normally the array would be filled in about 1 millisecond (as well), and the extra code adds between 0.3 and 0.8 milliseconds, on average 0.4ms.
Also of note: because the entire thing is cached to begin with, *we don't need to unserialize data on every page load*... As it's already unserialized through the cache. It's already the case in the current codebase.
That means, it only adds half a millisecond to performance *before* the cache enters into account. When caching is enabled (even at the default level, which is on by default), there is absolutely no difference in performance when retrieving arrays instead of variables.
I would thus strongly recommend adding support for arrays into updateSettings!
- Disabled caching for $settings
- Added unserialize directly to the $settings[$row[0]] = $row[1] line
- Result is about 40% slower, i.e. normally the array would be filled in about 1 millisecond (as well), and the extra code adds between 0.3 and 0.8 milliseconds, on average 0.4ms.
Also of note: because the entire thing is cached to begin with, *we don't need to unserialize data on every page load*... As it's already unserialized through the cache. It's already the case in the current codebase.
That means, it only adds half a millisecond to performance *before* the cache enters into account. When caching is enabled (even at the default level, which is on by default), there is absolutely no difference in performance when retrieving arrays instead of variables.
I would thus strongly recommend adding support for arrays into updateSettings!
4338
Features / Re: New revs - Public comments
« on October 8th, 2012, 05:59 PM »It doesn't make life any easier, on the contrary it makes plugin code more complicated because unless they're using a simple page (where we can hide the real complexity away from them), they have to do fudge things themselves. There are already examples in SMF and Wedge of doing crap like this (cf the warning settings, post moderation settings) only without being serialised, just comma-separated and that's messy enough.
I personally had a few times where I was going to store an array into $settings, and then thought to myself, "oh wait, I can't, I'm supposed to send a string instead..."
And if you're writing a plugin that accesses a serialized array in the settings, you don't want to have to unserialize it yourself, because when are you supposed to do that..? As a plugin author I'd expect Wedge to do it for me. (But then again that's just me...)
Imagine if you're accessing your $settings var in multiple places. Do you have to do an unserialize test on it everytime you use it..?
All it does is keep things marginally smaller in the database, it doesn't make anything easier per se. I'd honestly rather leave it alone because the extra complexity, the extra performance hit for a marginal (at best) saving of space with no improvement in convenience for anyone, and the potential to introduce new bugs unnecessarily puts me off. It is an interesting idea but it's got way more downsides than upsides.
As far as the sidebar, I'm not fussed either way, just whatever is most flexible for theme designers.
Posted: October 8th, 2012, 05:54 PM
Just did a quick test...
global $settings;
$t = microtime(true);
foreach ($settings as $truc)
if (strpos($truc, '{') !== false)
$truc = @unserialize($truc);
echo microtime(true)-$t;Returns in around 1 millisecond on my local setup. About 2 if I remove the strpos() test, meaning it's best to avoid calling unserialize for nothing.
Is that worth the delay then..?
:edit: At this point in the code, they're already unserialized. I should test for another...
4339
Features / Re: New revs - Public comments
« on October 8th, 2012, 05:00 PM »
Okay... Just had a couple of ideas I need to get opinions about.
- Loading settings: currently, we're getting strings, and for some we need to unserialize them, so we do that... I was thinking, if a plugin wants to save some settings, maybe we should recommend that they save them into a single variable because it's 'cleaner' that way, i.e. into an array... But because people might not want to do the serialize work, here's what we could do: we load $settings, and then iterate through it, searching for '{' in strings, if found, $arr = @unserialize($string), and if $arr !== false, then use $arr instead of the serialized string. Then, at updateSettings time, we can simply do an is_array() test, and if true, serialize the string, but still return the array instead of the serialized string of course.
What do you think...?
The only place where this would slow down performance is the strpos() on every single variable, or we could just directly try to unserialize them without a test, it warrants a test anyway... Other than that, it'd make life easier for everyone, I think.
- The sidebar... I've been postponing for a couple of weeks a skin change to Weaving that actually pushes the linktree inside the content area, because I figured that it's easier to get it outside of the content area with CSS (a negative margin, basically), than inside it. I've been trying to have Wuthering be the example of putting the linktree outside of the content area, but it doesn't work as well as I thought because the table-cell status of divs makes it impossible to adjust the sidebar without cancelling its table-cell status (which allows it to extend to the bottom of the page.)
So, basically, I have a few choices left...
- Give up on table-cell for the sidebar, like I suggested earlier, and use other tricks instead,
- Use a <skeleton> to move the linktree around in Wuthering, rather than a CSS change (this is probably what makes the most sense, but I wanted to show that it was possible to do with CSS...)
- Or just give up on putting the linktree above the sidebar & content, just have it sit above content and beside the sidebar as per usual.
What do you think...? (If anything.)
PS: I posted that to the wrong topic, but at least it reminds me that I have a couple of comments to make on the loadLanguage and capitalization changes...
- Loading settings: currently, we're getting strings, and for some we need to unserialize them, so we do that... I was thinking, if a plugin wants to save some settings, maybe we should recommend that they save them into a single variable because it's 'cleaner' that way, i.e. into an array... But because people might not want to do the serialize work, here's what we could do: we load $settings, and then iterate through it, searching for '{' in strings, if found, $arr = @unserialize($string), and if $arr !== false, then use $arr instead of the serialized string. Then, at updateSettings time, we can simply do an is_array() test, and if true, serialize the string, but still return the array instead of the serialized string of course.
What do you think...?
The only place where this would slow down performance is the strpos() on every single variable, or we could just directly try to unserialize them without a test, it warrants a test anyway... Other than that, it'd make life easier for everyone, I think.
- The sidebar... I've been postponing for a couple of weeks a skin change to Weaving that actually pushes the linktree inside the content area, because I figured that it's easier to get it outside of the content area with CSS (a negative margin, basically), than inside it. I've been trying to have Wuthering be the example of putting the linktree outside of the content area, but it doesn't work as well as I thought because the table-cell status of divs makes it impossible to adjust the sidebar without cancelling its table-cell status (which allows it to extend to the bottom of the page.)
So, basically, I have a few choices left...
- Give up on table-cell for the sidebar, like I suggested earlier, and use other tricks instead,
- Use a <skeleton> to move the linktree around in Wuthering, rather than a CSS change (this is probably what makes the most sense, but I wanted to show that it was possible to do with CSS...)
- Or just give up on putting the linktree above the sidebar & content, just have it sit above content and beside the sidebar as per usual.
What do you think...? (If anything.)
PS: I posted that to the wrong topic, but at least it reminds me that I have a couple of comments to make on the loadLanguage and capitalization changes...
4340
Off-topic / Re: Happy birthday, Arantor!
« on October 7th, 2012, 10:16 AM »Well, astrology is fascinating..but its rather useless IMHO if you only look at the sun-sign. People in same sign can be vastly different, depending on where their ascendant falls. Myself I am a Libra with a Sagittarius ascendant, which might make me seem possibly more like the latter than the former.
So, either astrology 'works' and there's a 2-hour delay in star sign calculation that we should notify the IAAOCA (International Astrology And Other Crap Association) about it, or people lied to me about the exact time of my birth, or it's just as I said: a load of bollocks that you tend to 'stick to' and modify your personality based on what they say of you.
Or something.
4341
Features / Re: New revs - Public comments
« on October 6th, 2012, 03:10 PM »I like the new tweaks, it looks nice :)
Yes, I agree that the installer test should be done like that - but let's go one better. If it doesn't pass for whatever reason, automatically disable the option.
But the problem is that as soon as I'm starting work on that, I'll get the idea to do the same for pretty URL support discovery (i.e. call for /whatever/ through Ajax and see if $.load() get something in return...), and I have a feeling I'll be spending a week on all of this... :^^;:
My personal goal is to have a 'releasable' alpha version by next week. (Which we'll release in private first anyway, but you get my point...)
Well, doing this is easy enough; wetem can have control over it. The one thing that it would mean is that it would be... difficult to inject the SVN version number the way you currently have it in the footer,
This is essentially the problem: whatever method we leave in for ourselves to use here will be exploitable elsewhere. But I think that's a compromise we can live with ;)
4342
Features / Re: New revs
« on October 6th, 2012, 03:01 PM »
rev 1732
(4 files, 1kb)
! Forgot to declare the @language keywords... Maybe I should assume that 'index' is always loaded anyway, at that point in the script..? (editor.js, post.js, topic.js)
- Duplicate CSS declaration. (Wine/extra.css)
(4 files, 1kb)
! Forgot to declare the @language keywords... Maybe I should assume that 'index' is always loaded anyway, at that point in the script..? (editor.js, post.js, topic.js)
- Duplicate CSS declaration. (Wine/extra.css)
4343
Features / Re: New revs
« on October 6th, 2012, 02:15 PM »
rev 1731
(8 files, 7kb)
* Moved index.css's variables to common.css, so that they can be re-used in external files. I'll commit the slim version of index.css once I'm done with all the little tweaks. Also updated .postbg background declarations to use $post_bg and $post_bg2. Which I could have sworn I'd already done... (common.css, sections.css, Wine/extra.css)
* Moved oEditorStrings and ptxt to editor.js and post.js respectively... Now that's even cooler. Changed opt.message_ext_error_final to a local variable. (Class-Editor.php, Post.template.php, editor.js, post.js)
* .zoom-base should be a virtual, even its name implies it... (zoom.css)
(8 files, 7kb)
* Moved index.css's variables to common.css, so that they can be re-used in external files. I'll commit the slim version of index.css once I'm done with all the little tweaks. Also updated .postbg background declarations to use $post_bg and $post_bg2. Which I could have sworn I'd already done... (common.css, sections.css, Wine/extra.css)
* Moved oEditorStrings and ptxt to editor.js and post.js respectively... Now that's even cooler. Changed opt.message_ext_error_final to a local variable. (Class-Editor.php, Post.template.php, editor.js, post.js)
* .zoom-base should be a virtual, even its name implies it... (zoom.css)
4344
Archived fixes / Re: Shortcode text is wrong in Firefox
« on October 6th, 2012, 07:35 AM »It's Shift + Alt + S to post. The text needs changed.
Always has been.
4345
Features / Re: New revs
« on October 5th, 2012, 09:54 PM »
rev 1730
(12 files, 4kb)
* Moved $txt['quickmod_confirm'] declaration to script.js, renamed it to $txt['generic_confirm_request'], and replaced everything everywhere... This is for that kind of cool simplification that I implemented cacheable language strings. :) Also simplified InTopicModeration strings. (ManageMail.php, Aeva-Foxy.php, Display.template.php, ManageMedia.template.php, Media.template.php, MessageIndex.template.php, Search.template.php, index.language.php, mediadmin.js, script.js, topic.js)
+ Added a confirm box to the thought removal button... After (as an admin) deleting several thoughts from wedge.org accidentally in the last few months, it was long overdue... (script.js)
(12 files, 4kb)
* Moved $txt['quickmod_confirm'] declaration to script.js, renamed it to $txt['generic_confirm_request'], and replaced everything everywhere... This is for that kind of cool simplification that I implemented cacheable language strings. :) Also simplified InTopicModeration strings. (ManageMail.php, Aeva-Foxy.php, Display.template.php, ManageMedia.template.php, Media.template.php, MessageIndex.template.php, Search.template.php, index.language.php, mediadmin.js, script.js, topic.js)
+ Added a confirm box to the thought removal button... After (as an admin) deleting several thoughts from wedge.org accidentally in the last few months, it was long overdue... (script.js)
4346
Features / Re: New revs
« on October 5th, 2012, 04:36 PM »
rev 1729
(3 files, 2kb)
* Tweaked default welcome page to look better with the current stylesheets.
Ensured that InfoCenter wouldn't use any hardcoded styles because it wouldn't be too good on a generic template.
Sidebar should have some bottom padding when it's at the bottom.
Finished converting font weights.
(InfoCenter.template.php, Welcome.template.php, sections.css)
(3 files, 2kb)
* Tweaked default welcome page to look better with the current stylesheets.
Ensured that InfoCenter wouldn't use any hardcoded styles because it wouldn't be too good on a generic template.
Sidebar should have some bottom padding when it's at the bottom.
Finished converting font weights.
(InfoCenter.template.php, Welcome.template.php, sections.css)
4347
Features / Re: New revs - Public comments
« on October 5th, 2012, 08:57 AM »
@live> Generally speaking, you might want to be careful with your current local installs everytime you see a commit that modifies the other/ folder, especially the install.* and Settings files ;)
So, guys, what do you think about the new installer tweaks...?
Apart from the design, I'm also considering doing something smart: launching the Ajax test automatically instead of upon a click, and showing the results directly in the current window, i.e. a good opportunity to use $().load()... :P
@Pete & Sara> Hmm... So, let's sort it out: instead of trusting modders not to add links carelessly in the footer, we should prevent them from doing so at all. This goes through the use of private vars. We could possibly even test for the length of outputted strings, and if it's too long or its base64 version or whatever doesn't have a specific bit of string in it we can reset it, blah blah blah... (I'm trying to stick to trusting people, but I guess we can go either way eh..? :^^;:)
So, guys, what do you think about the new installer tweaks...?
Apart from the design, I'm also considering doing something smart: launching the Ajax test automatically instead of upon a click, and showing the results directly in the current window, i.e. a good opportunity to use $().load()... :P
@Pete & Sara> Hmm... So, let's sort it out: instead of trusting modders not to add links carelessly in the footer, we should prevent them from doing so at all. This goes through the use of private vars. We could possibly even test for the length of outputted strings, and if it's too long or its base64 version or whatever doesn't have a specific bit of string in it we can reset it, blah blah blah... (I'm trying to stick to trusting people, but I guess we can go either way eh..? :^^;:)
4348
Features / Re: New revs
« on October 5th, 2012, 12:07 AM »
rev 1728 -- more files left to commit -- but not too many actually! I'm getting close to a releasable alpha really...
(12 files, 2kb)
* Harmonized font-weight declarations to 400 and 700 instead of normal and bold. It's a tad shorter, and skinners really should learn to use weights by number rather than by name. It makes more sense in the end. Especially for the non-English population... I also updated section.css but need to clean it up first. (editor.js, install.css, extra.rtl.css, index.member.css, mana.css, media.css, report.css, up.css, zoom.css, Wine/extra.css, Warm/extra.css, Wuthering/extra.css)
:edit: Had got the commit number wrong...
(12 files, 2kb)
* Harmonized font-weight declarations to 400 and 700 instead of normal and bold. It's a tad shorter, and skinners really should learn to use weights by number rather than by name. It makes more sense in the end. Especially for the non-English population... I also updated section.css but need to clean it up first. (editor.js, install.css, extra.rtl.css, index.member.css, mana.css, media.css, report.css, up.css, zoom.css, Wine/extra.css, Warm/extra.css, Wuthering/extra.css)
:edit: Had got the commit number wrong...
4349
Features / Re: New revs
« on October 4th, 2012, 06:46 PM »
rev 1727 -- the installer overhaul. Still got files left to commit, but nothing related to the installer I believe.
(9 files, 13kb)
! Fixed clean_cache() to properly handle sub-folders and delete them if they're emptied. (Subs-Cache.php)
! Fixed updateSettings()'s handling during install. (install.php, Load.php, Subs.php)
! Fixed database structure, got screwed up a bit last month. (install.sql)
! enableCompressedData was never set up at install time, despite all the related code. (install.php, install.sql)
! Settings.php file was being rewritten with superfluous linebreaks everywhere. (install.php)
! Fixed CSS cache file generation in install process, it was broken for multiple reasons. (install.php)
! Fixed 404 handling to support the new css and js cache folders. (QueryString.php)
* Overhauled installer design. It's looking quite a bit better now. (install.php, install.css)
* Moved language select box from header to first page's content area. Also fixed the JS to actually use sbox on it. (install.php)
* Improved the 'pass' test strings during install. (Install.language.php)
(9 files, 13kb)
! Fixed clean_cache() to properly handle sub-folders and delete them if they're emptied. (Subs-Cache.php)
! Fixed updateSettings()'s handling during install. (install.php, Load.php, Subs.php)
! Fixed database structure, got screwed up a bit last month. (install.sql)
! enableCompressedData was never set up at install time, despite all the related code. (install.php, install.sql)
! Settings.php file was being rewritten with superfluous linebreaks everywhere. (install.php)
! Fixed CSS cache file generation in install process, it was broken for multiple reasons. (install.php)
! Fixed 404 handling to support the new css and js cache folders. (QueryString.php)
* Overhauled installer design. It's looking quite a bit better now. (install.php, install.css)
* Moved language select box from header to first page's content area. Also fixed the JS to actually use sbox on it. (install.php)
* Improved the 'pass' test strings during install. (Install.language.php)
4350
Archived fixes / Re: Error message on search
« on October 4th, 2012, 05:57 PM »
Good then, I've uploaded the new version and it doesn't trigger the error any more. :)