Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use %uvector-replace in replace even when subseqs differ in length #529

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

se-mz
Copy link
Contributor

@se-mz se-mz commented Mar 19, 2025

This significantly speeds up code that relies on the way replace implicitly intersects sequence bounds. For example, the replace calls below take about 150ms & 3ms respectively before the patch; both take about 3ms afterwards.

(let ((buf  (make-array 100000000 :element-type '(unsigned-byte 8)))
      (buf2 (make-array 10000000  :element-type '(unsigned-byte 8) :initial-element 1)))
  (time (replace buf buf2))
  (time (replace buf buf2 :start1 0 :end1 (length buf2)))
  nil)

Originally found by Gilbert Baumann a while ago.

This significantly speeds up code that relies on the way replace implicitly
intersects sequence bounds. The removed listp check is redundant.
@xrme
Copy link
Member

xrme commented Mar 19, 2025

In the case of something like
(make-array 100000000 :element-type '(unsigned-byte 8)), where the memory is implicitly zero-filled (in CCL, anyway), it is possible that the memory will not actually be "real" yet, and that therefore the make-array call will take very little time. (The memory will be provided zero-filled on demand pages from the operating system when we're consing into fresh memory.)

In a fresh lisp. you'll almost certainly see a time difference between (make-array 100000000 :element-type '(unsigned-byte 8)) and (make-array 100000000 :element-type '(unsigned-byte 8) :initial-element 1).

Your improvement to replace is probably a win anyway, but I wanted you to be aware of a possible source of timing error because you might accidentally be measuring the time needed for the operating system to provide zero-filled-on-demand pages for buf in the first replace form.

@se-mz
Copy link
Contributor Author

se-mz commented Mar 19, 2025

Good catch, but it seems like that doesn't matter here; I get comparable readings when initializing both with non-zero values.

@xrme xrme merged commit cc83449 into Clozure:master Mar 19, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants