From: Ihor Radchenko <yantar92@gmail.com>
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Re: tangle option to not write a file with same contents?
Date: Fri, 03 Jun 2022 15:04:14 +0800 [thread overview]
Message-ID: <87mteucjo1.fsf@localhost> (raw)
In-Reply-To: <t75efi$9pv$1@ciao.gmane.io>
Max Nikulin <manikulin@gmail.com> writes:
> On 30/05/2022 10:14, Ihor Radchenko wrote:
>> I applied the discussed two patches onto main via 1525a5a64 and
>> f6f26d4ce. The suggested amendments were incorporated.
>
> So Greg's feature request is implemented and it is great.
>
> I am less confident with `org-babel-load-file' though. Due to poor error
> handling in `org-babel-tangle' & Co., I am unsure that I can provide an
> example when new code incorrectly updates file modification time despite
> tangle failure, but I do not like that modification time is changed
> before attempt to tangle. Generally I expected that it is performed
> after tangling, perhaps with check that `org-babel-tangle-file' returned
> expected file name (as some hope for success).
I agree that updating the modification time after tangling will be
slightly more robust in common cases. Though not necessarily strictly
better.
Handling errors is challenging. org-babel-tangle potentially writes to
multiple files in unspecified order. Tangle error may happen in the
middle of tangling when some files are written and some are not. Those
tangled files could also depend on each other during loading (think of
config.org tangling into init.el and config.el one requiring another).
Then, an error during tangling can result in some files being tangled to
newer versions and some still having old versions.
Now, consider that config.org is tangled to config.el and then to
init.el with the latter tangling process failing. Next time we call
org-babel-load-file, the updated config.el will be loaded despite error
and incorrectly require the old init.el.
Another case is when init.el is tangled first and config.el tangling
fails. This time, it matters whether we update the modification time of
config.el (before of after tangling). In our current code (with my
patch), the older version of config.el will be touched despite tangling
error and next time old config.el + new init.el will be loaded.
The existing code will create problems in both the described cases.
If we update file modification time only after tangling succeeds without
error, only the first case will be problematic.
So, your proposal will indeed be more robust in a sense that it will
prevent _some_ problems when tangling fails.
However, I see the "robustness" as a bad thing here. org-babel-load will
sometimes fail and sometimes not depending on the where in config.org
the error appears. I see it as a potential nightmare for debugging.
Ideally, we should solve the problem with tangle errors in some other
way. I am not sure how (or rather haven't spent much time thinking).
Ideas are welcome!
> I have realized that the case was not exactly the same when Nicolas
> asked for a patch correction:
>
> Nicolas Goaziou to emacs-orgmode. Re: [PATCH] Fix moving cursor in
> org-set-tags-command. Fri, 08 May 2020 09:53:55 +0200.
> https://list.orgmode.org/87ftca6ewc.fsf@nicolasgoaziou.fr
>
> "Please replace `and' with `when' if side-effects are involved."
I see. `and' is indeed considered a logical statement and should not
normally be abused for side-effects. But it does not imply the opposite
for `when'.
>> Everything is tangled to files prior to running the benchmark. This way,
>> none of the tangled files should change when calling org-babel-tangle
>> and no disk caches should be involved.
>
> I am trying to say that in your benchmark file are not read from disk,
> almost certainly they are cached in memory. Besides write cache, while
> reading, content may be taken from cache as well.
>
> Actually I am just speculating, while you have benchmark result. It is
> unlikely that I will decide to spend some time arranging another test to
> convince you with numbers, priority of such activity is not high enough.
Let me clarify. I am looking at this benchmark from a broader
perspective:
- We want the patch to improve performance.
- We don't want the patch to degrade performance.
1. My benchmark certainly shows that there is an improvement in some
cases. It is something we want.
2. My benchmark may not be accurate depending on the system state. It is
also fine. Just shows that improvement might not happen in 100% of
cases.
3. Is my benchmark degrading performance? I do not see how and I do not
see how your observations prove otherwise. So, unless you can see how
my patch degrades performance on some systems, I do not see any
reason to perform more accurate benchmarking other than for science.
Best,
Ihor
next prev parent reply other threads:[~2022-06-03 7:29 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-28 4:04 tangle option to not write a file with same contents? Greg Minshall
2021-10-29 16:21 ` Max Nikulin
2021-10-29 17:58 ` Greg Minshall
2021-10-30 15:13 ` Max Nikulin
2021-10-30 16:13 ` Greg Minshall
2022-05-07 8:05 ` Max Nikulin
2022-05-08 4:42 ` [PATCH] " Ihor Radchenko
2022-05-17 15:39 ` Max Nikulin
2022-05-30 3:14 ` Ihor Radchenko
2022-05-31 16:07 ` Max Nikulin
2022-06-03 7:04 ` Ihor Radchenko [this message]
2022-06-07 3:47 ` Tom Gillespie
2022-06-01 13:18 ` Greg Minshall
2022-09-12 17:36 ` Org version mismatch -- hooray! Greg Minshall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mteucjo1.fsf@localhost \
--to=yantar92@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=manikulin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.