Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
* level limiting condition moved to the macro as requested by Simo, * macro definition update is separated from the invocation update, * automatic macro invocation update is separated from the following manual fixups, * invocations using the old debug level (number literals) are updated to use the new bitmask macros, * the level conversion function "debug_get_level" is removed.
Please feel free to pick and squash commits as necessary.
Sincerely, Nick
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 12/20/2013 11:15 AM, Nikolai Kondrashov wrote:
Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
- level limiting condition moved to the macro as requested by
Simo, * macro definition update is separated from the invocation update, * automatic macro invocation update is separated from the following manual fixups, * invocations using the old debug level (number literals) are updated to use the new bitmask macros, * the level conversion function "debug_get_level" is removed.
Please feel free to pick and squash commits as necessary.
Sincerely, Nick
Patch 0001: Ack Patch 0002: Ack Patch 0003: Ack Patch 0004: Ack
Patch 0005: Nack Patch 0006: Nack
I'd rather that 0005 and 0006 be squashed together and that the comment be written as follows:
{{{ Make DEBUG macro invocations variadic
Use a script to update DEBUG macro invocations to use it as a variadic macro, supplying format string and its arguments directly, instead of wrapping them in parens.
This script was used to update the code:
grep -rwl --include '*.[hc]' DEBUG . | while read f; do mv "$f"{,.orig} perl -e \ 'use strict; use File::Slurp; my $text=read_file(*STDIN); $text=~s#(\bDEBUG\s*([^(]+)((.*?))\s*)\s*;#$1$2);#gs; print $text;' < "$f.orig" > "$f" rm "$f.orig" done
There exists a single non-automated change in this patch, due to the script being slightly too greedy in two places.
1. The definition of the DEBUG() macro matched incorrectly, causing the parser to greedily continue advancing until it eventually found a double-closing-parenthesis in the talloc_zfree() macro definition.
2. The presence of a comment containing the literal string "DEBUG()" caused the parser to incrorectly match and proceed to the next valid DEBUG() macro usage, removing the trailing parenthesis but missing the leading one. }}}
That being said, the logic seems correct everywhere else. Nikolai, if you agree with this, no need to resend the patches. This is easily adjusted before we push, so you can consider it an Ack.
Patch 0007: The conversion seems *correct*, if not ideal. I don't love that it effectively makes all of our DEBUG lines exceed our 79-character limit. I don't suppose you could amend your patch to reflow things using our current convention:
DEBUG(SSSDBG_TRACE_ALL, "Some words go here\n", arg1, arg2);
I'm not willing to block the patch on this, however. Particularly since a non-trivial number of existing DEBUG messages were already over the limit.
One other comment I seem to recall from the earlier (long) thread that I'd like to address. It was brought up that the reason we were leaving these alone was to deal with them only when we touched the code they to which they were related. I think by doing this mass-conversion, it becomes much easier to spot at a glance when a message is poorly categorized. It makes it possible for a new contributor to come in and fix up these levels, even when not working on other code in the area.
So I'm in favor of making this change, and best to do it all at once and rip off the band-aid.
Consider 0007 acked, unless Nikolai wants to try his hand at reflowing it.
Patch 0008: I'm not sure we want to drop this compatibility without checking with all of our downstream packagers (and anyone who is building with custom patches). It's very possible that they may be carrying forward old patches that still retain the 0-9 debug level values and breaking them might be unwelcome.
At the very least, this would need to be documented loudly in the release notes for the version in which this change occurs.
So, technical Ack, with reservations.
I've tested that all of these patches work as expected and that no functionality appears to be lost in the debug logging.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 12/20/2013 02:31 PM, Stephen Gallagher wrote:
On 12/20/2013 11:15 AM, Nikolai Kondrashov wrote:
Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
- level limiting condition moved to the macro as requested by
Simo, * macro definition update is separated from the invocation update, * automatic macro invocation update is separated from the following manual fixups, * invocations using the old debug level (number literals) are updated to use the new bitmask macros, * the level conversion function "debug_get_level" is removed.
Please feel free to pick and squash commits as necessary.
Sincerely, Nick
Patch 0001: Ack Patch 0002: Ack Patch 0003: Ack Patch 0004: Ack
Patch 0005: Nack Patch 0006: Nack
I'd rather that 0005 and 0006 be squashed together and that the comment be written as follows:
{{{ Make DEBUG macro invocations variadic
Use a script to update DEBUG macro invocations to use it as a variadic macro, supplying format string and its arguments directly, instead of wrapping them in parens.
This script was used to update the code:
grep -rwl --include '*.[hc]' DEBUG . | while read f; do mv "$f"{,.orig} perl -e \ 'use strict; use File::Slurp; my $text=read_file(*STDIN); $text=~s#(\bDEBUG\s*([^(]+)((.*?))\s*)\s*;#$1$2);#gs; print $text;' < "$f.orig" > "$f" rm "$f.orig" done
There exists a single non-automated change in this patch, due to the script being slightly too greedy in two places.
- The definition of the DEBUG() macro matched incorrectly, causing
the parser to greedily continue advancing until it eventually found a double-closing-parenthesis in the talloc_zfree() macro definition.
- The presence of a comment containing the literal string
"DEBUG()" caused the parser to incrorectly match and proceed to the next valid DEBUG() macro usage, removing the trailing parenthesis but missing the leading one. }}}
That being said, the logic seems correct everywhere else. Nikolai, if you agree with this, no need to resend the patches. This is easily adjusted before we push, so you can consider it an Ack.
Patch 0007: The conversion seems *correct*, if not ideal. I don't love that it effectively makes all of our DEBUG lines exceed our 79-character limit. I don't suppose you could amend your patch to reflow things using our current convention:
DEBUG(SSSDBG_TRACE_ALL, "Some words go here\n", arg1, arg2);
I'm not willing to block the patch on this, however. Particularly since a non-trivial number of existing DEBUG messages were already over the limit.
One other comment I seem to recall from the earlier (long) thread that I'd like to address. It was brought up that the reason we were leaving these alone was to deal with them only when we touched the code they to which they were related. I think by doing this mass-conversion, it becomes much easier to spot at a glance when a message is poorly categorized. It makes it possible for a new contributor to come in and fix up these levels, even when not working on other code in the area.
So I'm in favor of making this change, and best to do it all at once and rip off the band-aid.
Consider 0007 acked, unless Nikolai wants to try his hand at reflowing it.
Patch 0008: I'm not sure we want to drop this compatibility without checking with all of our downstream packagers (and anyone who is building with custom patches). It's very possible that they may be carrying forward old patches that still retain the 0-9 debug level values and breaking them might be unwelcome.
At the very least, this would need to be documented loudly in the release notes for the version in which this change occurs.
Actually, minor Nack on 0008:
#define DEBUG(level, format, ...) do { \ - - int __debug_macro_newlevel = debug_get_level(level); \ - - if (DEBUG_IS_SET(__debug_macro_newlevel)) \ - - debug_fn(__FUNCTION__, __debug_macro_newlevel, format, ##__VA_ARGS__); \ + int __debug_macro_level = level; \ + if (DEBUG_IS_SET(__debug_macro_level)) \ + debug_fn(__FUNCTION__, __debug_macro_level, format, ##__VA_ARGS__); \ } while (0)
If you're going to drop debug_get_level(), there's no reason to use _debug_macro_level at all here. Drop that too.
On 12/20/2013 09:43 PM, Stephen Gallagher wrote:
Actually, minor Nack on 0008:
#define DEBUG(level, format, ...) do { \
- int __debug_macro_newlevel = debug_get_level(level); \
- if (DEBUG_IS_SET(__debug_macro_newlevel)) \
debug_fn(__FUNCTION__, __debug_macro_newlevel, format,
##__VA_ARGS__); \
- int __debug_macro_level = level; \
- if (DEBUG_IS_SET(__debug_macro_level)) \
debug_fn(__FUNCTION__, __debug_macro_level, format,
##__VA_ARGS__); \ } while (0)
If you're going to drop debug_get_level(), there's no reason to use _debug_macro_level at all here. Drop that too.
Initially I removed it, but then reverted. If removed, it will be possible for "level" to get evaluated thrice (twice in DEBUG_IS_SET), which might be undesirable.
Sincerely, Nick
Hi Stephen,
Thanks for the review! Replies below.
On 12/20/2013 09:31 PM, Stephen Gallagher wrote:
I'd rather that 0005 and 0006 be squashed together and that the comment be written as follows:
{{{ Make DEBUG macro invocations variadic
Use a script to update DEBUG macro invocations to use it as a variadic macro, supplying format string and its arguments directly, instead of wrapping them in parens.
This script was used to update the code:
grep -rwl --include '*.[hc]' DEBUG . | while read f; do mv "$f"{,.orig} perl -e \ 'use strict; use File::Slurp; my $text=read_file(*STDIN); $text=~s#(\bDEBUG\s*([^(]+)((.*?))\s*)\s*;#$1$2);#gs; print $text;' < "$f.orig" > "$f" rm "$f.orig" done
There exists a single non-automated change in this patch, due to the script being slightly too greedy in two places.
- The definition of the DEBUG() macro matched incorrectly, causing the
parser to greedily continue advancing until it eventually found a double-closing-parenthesis in the talloc_zfree() macro definition.
- The presence of a comment containing the literal string "DEBUG()"
caused the parser to incrorectly match and proceed to the next valid DEBUG() macro usage, removing the trailing parenthesis but missing the leading one. }}}
That being said, the logic seems correct everywhere else. Nikolai, if you agree with this, no need to resend the patches. This is easily adjusted before we push, so you can consider it an Ack.
Sure, these were made separate with the intention to make everyone aware that the script was non-ideal and to make it easier to re-run it if the code changes before it is acked. It is better to submit these as a single commit and the message above will do fine.
Patch 0007: The conversion seems *correct*, if not ideal. I don't love that it effectively makes all of our DEBUG lines exceed our 79-character limit. I don't suppose you could amend your patch to reflow things using our current convention:
DEBUG(SSSDBG_TRACE_ALL, "Some words go here\n", arg1, arg2);
I'm not willing to block the patch on this, however. Particularly since a non-trivial number of existing DEBUG messages were already over the limit.
One other comment I seem to recall from the earlier (long) thread that I'd like to address. It was brought up that the reason we were leaving these alone was to deal with them only when we touched the code they to which they were related. I think by doing this mass-conversion, it becomes much easier to spot at a glance when a message is poorly categorized. It makes it possible for a new contributor to come in and fix up these levels, even when not working on other code in the area.
So I'm in favor of making this change, and best to do it all at once and rip off the band-aid.
Consider 0007 acked, unless Nikolai wants to try his hand at reflowing it.
Yes, I wanted to comment on line lengths, but then forgot about it, sorry.
It might be tricky to do automatically and might harm more if done incorrectly, but let me try something. Maybe I'll be able to improve it for most cases.
Patch 0008: I'm not sure we want to drop this compatibility without checking with all of our downstream packagers (and anyone who is building with custom patches). It's very possible that they may be carrying forward old patches that still retain the 0-9 debug level values and breaking them might be unwelcome.
At the very least, this would need to be documented loudly in the release notes for the version in which this change occurs.
So, technical Ack, with reservations.
Hmm, yes it might happen. I'll leave this up to you, of course.
Sincerely, Nick
On 12/20/2013 10:01 PM, Nikolai Kondrashov wrote:
On 12/20/2013 09:31 PM, Stephen Gallagher wrote:
Consider 0007 acked, unless Nikolai wants to try his hand at reflowing it.
Yes, I wanted to comment on line lengths, but then forgot about it, sorry.
It might be tricky to do automatically and might harm more if done incorrectly, but let me try something. Maybe I'll be able to improve it for most cases.
Alright, please find attached a reflowed version of 0007 "Update DEBUG* invocations to use new levels".
This time only 20 resulting lines are over 80 characters and only those that were too long before.
I should have tried to do this right from the start, but hurried it.
Sincerely, Nick
On (20/12/13 18:15), Nikolai Kondrashov wrote:
Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
- level limiting condition moved to the macro as requested by Simo,
- macro definition update is separated from the invocation update,
- automatic macro invocation update is separated from the following manual fixups,
- invocations using the old debug level (number literals) are updated to use the new bitmask macros,
I am not really sure, whether we need to convert old debug levels to new debug levels with script.
I don't remember what was the result in previous thread. Let's discuss about this point one more time.
When I replaced some debug levels in the past, I had to change level, because it didn't fit the description from manual pages.
man sssd.conf -> SERVICES SECTIONS -> General service configuration options -> debug_level
I can see two options 1. change debug level with Nikolai's script and occasionally update debug levels to the proper level 2. Push patches without this part and continue with occasionally conversions
LS
On 12/21/2013 08:47 PM, Lukas Slebodnik wrote:
On (20/12/13 18:15), Nikolai Kondrashov wrote:
Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
* level limiting condition moved to the macro as requested by Simo, * macro definition update is separated from the invocation update, * automatic macro invocation update is separated from the following manual fixups, * invocations using the old debug level (number literals) are updated to use the new bitmask macros,
I am not really sure, whether we need to convert old debug levels to new debug levels with script.
I don't remember what was the result in previous thread. Let's discuss about this point one more time.
When I replaced some debug levels in the past, I had to change level, because it didn't fit the description from manual pages.
man sssd.conf -> SERVICES SECTIONS -> General service configuration options -> debug_level
I can see two options
- change debug level with Nikolai's script and occasionally update debug levels to the proper level
- Push patches without this part and continue with occasionally conversions
The first option is better for the following reasons:
1. There are 2273 uses of the old levels, *half* of all DEBUG macro invocations, so updating them on occasion will take a *long* time. 2. Code is more often read than written (modified), and all this time developers will be reading code using obscure numeric literals, which take additional mental hop (effort) to comprehend. 3. Spotting incorrect debug level will be easier with symbolic levels instead of numeric ones and thus they're more likely to be corrected.
Sincerely, Nick
On Sun, Dec 22, 2013 at 11:23:58AM +0200, Nikolai Kondrashov wrote:
On 12/21/2013 08:47 PM, Lukas Slebodnik wrote:
On (20/12/13 18:15), Nikolai Kondrashov wrote:
Hi everyone,
Please find attached the third version of the DEBUG macro refactoring patchset. The second version was lost to the maillist message size limit, so I'll list the changes from the first one:
- level limiting condition moved to the macro as requested by Simo,
- macro definition update is separated from the invocation update,
- automatic macro invocation update is separated from the following manual fixups,
- invocations using the old debug level (number literals) are updated to use the new bitmask macros,
I am not really sure, whether we need to convert old debug levels to new debug levels with script.
I don't remember what was the result in previous thread. Let's discuss about this point one more time.
When I replaced some debug levels in the past, I had to change level, because it didn't fit the description from manual pages.
man sssd.conf -> SERVICES SECTIONS -> General service configuration options -> debug_level
I can see two options
- change debug level with Nikolai's script and occasionally update debug levels to the proper level
- Push patches without this part and continue with occasionally conversions
The first option is better for the following reasons:
I mostly agree with Lukas, but per conversation on #sssd the other developers preferred to mass-change the debug logs and I don't want to block the patches because of this.
1. There are 2273 uses of the old levels, *half* of all DEBUG macro invocations, so updating them on occasion will take a *long* time. 2. Code is more often read than written (modified), and all this time developers will be reading code using obscure numeric literals, which take additional mental hop (effort) to comprehend. 3. Spotting incorrect debug level will be easier with symbolic levels instead of numeric ones and thus they're more likely to be corrected.
Sorry, I disagree about 2) and 3). Maybe it's just how I work, but I've trained myself to convert any numeric constants I see in the code and just skip the SSSDBG_ unless I'm also touching the DEBUG macro for other reasons.
On 12/23/2013 01:42 PM, Jakub Hrozek wrote:
On Sun, Dec 22, 2013 at 11:23:58AM +0200, Nikolai Kondrashov wrote:
1. There are 2273 uses of the old levels, *half* of all DEBUG macro invocations, so updating them on occasion will take a *long* time. 2. Code is more often read than written (modified), and all this time developers will be reading code using obscure numeric literals, which take additional mental hop (effort) to comprehend. 3. Spotting incorrect debug level will be easier with symbolic levels instead of numeric ones and thus they're more likely to be corrected.
Sorry, I disagree about 2) and 3). Maybe it's just how I work, but I've trained myself to convert any numeric constants I see in the code and just skip the SSSDBG_ unless I'm also touching the DEBUG macro for other reasons.
The new debug levels were introduced at the beginning of August 2011 (a32ae272). That makes it about 2.5 years ago. Even assuming update progress is linear (which it isn't), it will take 2.5 years more to update the other half. That means 2.5 years of reading code with extra effort for developers. Although, admittedly, the less old code there is, the less effort there will be (but that is not linear as well).
Sincerely, Nick
On Wed, Dec 25, 2013 at 02:10:20AM +0200, Nikolai Kondrashov wrote:
On 12/23/2013 01:42 PM, Jakub Hrozek wrote:
On Sun, Dec 22, 2013 at 11:23:58AM +0200, Nikolai Kondrashov wrote:
1. There are 2273 uses of the old levels, *half* of all DEBUG macro invocations, so updating them on occasion will take a *long* time. 2. Code is more often read than written (modified), and all this time developers will be reading code using obscure numeric literals, which take additional mental hop (effort) to comprehend. 3. Spotting incorrect debug level will be easier with symbolic levels instead of numeric ones and thus they're more likely to be corrected.
Sorry, I disagree about 2) and 3). Maybe it's just how I work, but I've trained myself to convert any numeric constants I see in the code and just skip the SSSDBG_ unless I'm also touching the DEBUG macro for other reasons.
The new debug levels were introduced at the beginning of August 2011 (a32ae272). That makes it about 2.5 years ago. Even assuming update progress is linear (which it isn't), it will take 2.5 years more to update the other half. That means 2.5 years of reading code with extra effort for developers. Although, admittedly, the less old code there is, the less effort there will be (but that is not linear as well).
Sincerely, Nick
I spoke about the same problem with Simo (or Stephen? Sorry, I forgot over holidays) and it seems my opinion is in minority so I won't block the patchset over it.
I have some very minor comments about the patches, see below:
[PATCH 1/8] Add cscope inverted index files to .gitignore ACK
[PATCH 2/8] Move DEBUG macro body to debug_fn Mostly ACK, I just have a question about debug_fn() -- while I realize the code was taken from the old DEBUG macro, now that it's a function, maybe you could reflow it so it's more in line with our coding style and better readable as a result. In particular, there could be a newline after declaration of "va_list ap" and before call to "va_start".
Similarly, the block executed after testing debug_timestamps() could be its own functions, as in general we don't do intermixed code and declarations.
But this is mostly my personal preference, again, not blocking the patches.
[PATCH 3/8] Remove extra flushing from debug message output The code is fine, but why is it a separate patch and not part of #2 ? It seems the issue was introduced in patch 2/8, right?
[PATCH 4/8] Make DEBUG macro definition variadic ACK
[PATCH 5/8] Make DEBUG macro invocations variadic ACK, although I haven't reviewed the patch too closely, I simply checked couple of random messages.
[PATCH 6/8] Fixup DEBUG macro invocations update ACK
[PATCH 7/8] Update DEBUG* invocations to use new levels ACK (I tested the new version that was sent separately)
[PATCH 8/8] Remove debug_get_level as unnecessary ACK
I'm not too afraid about breaking 3rd party patches. We can announce this change on the devel list separately (w/o the [PATCH] keyword) and the patches would break at build-time, so the breakage wouldn't affect users, only packagers. They should be able to come for help if they can't fix the patch themselves.
So in conclusion - ACK, I'll leave up to you whether you want to change formatting of debug_fn and/or squash 3/8 into 2/8.
Great work!
Hi Jakub,
Thanks a lot for reviewing!
On 01/07/2014 02:33 PM, Jakub Hrozek wrote:
I spoke about the same problem with Simo (or Stephen? Sorry, I forgot over holidays) and it seems my opinion is in minority so I won't block the patchset over it.
I have some very minor comments about the patches, see below:
[PATCH 2/8] Move DEBUG macro body to debug_fn Mostly ACK, I just have a question about debug_fn() -- while I realize the code was taken from the old DEBUG macro, now that it's a function, maybe you could reflow it so it's more in line with our coding style and better readable as a result. In particular, there could be a newline after declaration of "va_list ap" and before call to "va_start".
Similarly, the block executed after testing debug_timestamps() could be its own functions, as in general we don't do intermixed code and declarations.
But this is mostly my personal preference, again, not blocking the patches.
I think a clearer approach would be to reflow it in a separate patch. I'll see if I have time for this. I wouldn't mind someone else fixing it, though.
[PATCH 3/8] Remove extra flushing from debug message output The code is fine, but why is it a separate patch and not part of #2 ? It seems the issue was introduced in patch 2/8, right?
No, it was there before my changes, in the original "debug_fn" function, which body was moved to the new "debug_vprintf". That's why I kept it separate. However, please squash it, if you think it's right.
So in conclusion - ACK, I'll leave up to you whether you want to change formatting of debug_fn and/or squash 3/8 into 2/8.
Great work!
Thanks Jakub. I'm still open to job offers ;)
Sincerely, Nick
On Tue, Jan 07, 2014 at 03:31:50PM +0200, Nikolai Kondrashov wrote:
Hi Jakub,
Thanks a lot for reviewing!
On 01/07/2014 02:33 PM, Jakub Hrozek wrote:
I spoke about the same problem with Simo (or Stephen? Sorry, I forgot over holidays) and it seems my opinion is in minority so I won't block the patchset over it.
I have some very minor comments about the patches, see below:
[PATCH 2/8] Move DEBUG macro body to debug_fn Mostly ACK, I just have a question about debug_fn() -- while I realize the code was taken from the old DEBUG macro, now that it's a function, maybe you could reflow it so it's more in line with our coding style and better readable as a result. In particular, there could be a newline after declaration of "va_list ap" and before call to "va_start".
Similarly, the block executed after testing debug_timestamps() could be its own functions, as in general we don't do intermixed code and declarations.
But this is mostly my personal preference, again, not blocking the patches.
I think a clearer approach would be to reflow it in a separate patch. I'll see if I have time for this. I wouldn't mind someone else fixing it, though.
[PATCH 3/8] Remove extra flushing from debug message output The code is fine, but why is it a separate patch and not part of #2 ? It seems the issue was introduced in patch 2/8, right?
No, it was there before my changes, in the original "debug_fn" function, which body was moved to the new "debug_vprintf". That's why I kept it separate. However, please squash it, if you think it's right.
Ah, sorry, my review was flawed. You're right.
Nevertheless, I think I found one bug in patch #2 when testing Stephen's follow-up patches. I don't think it's correct to replace all occurences of DEBUG_MSG with debug_fn. The reason is that DEBUG_MSG used to perform check if a particular level is set, debug_fn prints directly.
One example of where we need to check the debug level is ldb_debug_messages(), with the current version of the patch, the ldb messages are always printed.
On 01/07/2014 04:53 PM, Jakub Hrozek wrote:
On Tue, Jan 07, 2014 at 03:31:50PM +0200, Nikolai Kondrashov wrote:
On 01/07/2014 02:33 PM, Jakub Hrozek wrote:
[PATCH 3/8] Remove extra flushing from debug message output The code is fine, but why is it a separate patch and not part of #2 ? It seems the issue was introduced in patch 2/8, right?
No, it was there before my changes, in the original "debug_fn" function, which body was moved to the new "debug_vprintf". That's why I kept it separate. However, please squash it, if you think it's right.
Ah, sorry, my review was flawed. You're right.
Nevertheless, I think I found one bug in patch #2 when testing Stephen's follow-up patches. I don't think it's correct to replace all occurences of DEBUG_MSG with debug_fn. The reason is that DEBUG_MSG used to perform check if a particular level is set, debug_fn prints directly.
One example of where we need to check the debug level is ldb_debug_messages(), with the current version of the patch, the ldb messages are always printed.
You're right. This is a regression. I forgot to update that when I moved the level limiting back to the macro. Will fix.
Sincerely, Nick
sssd-devel@lists.fedorahosted.org