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

lib/fs: Optimize WindowsInvalidFilename #8687

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

greatroar
Copy link
Contributor

Replaced strings.Split with the new strings.Cut, which doesn't allocate.

name                           old time/op    new time/op    delta
WindowsInvalidFilenameValid-8     154ns ± 2%      89ns ± 0%   -42.09%  (p=0.000 n=10+9)
WindowsInvalidFilenameNUL-8       124ns ± 2%     124ns ± 1%      ~     (p=0.371 n=8+10)

name                           old alloc/op   new alloc/op   delta
WindowsInvalidFilenameValid-8     16.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)
WindowsInvalidFilenameNUL-8       19.0B ± 0%      3.0B ± 0%   -84.21%  (p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
WindowsInvalidFilenameValid-8      1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
WindowsInvalidFilenameNUL-8        2.00 ± 0%      1.00 ± 0%   -50.00%  (p=0.000 n=10+10)

Replaced strings.Split with the new strings.Cut, which doesn't allocate.

name                           old time/op    new time/op    delta
WindowsInvalidFilenameValid-8     154ns ± 2%      89ns ± 0%   -42.09%  (p=0.000 n=10+9)
WindowsInvalidFilenameNUL-8       124ns ± 2%     124ns ± 1%      ~     (p=0.371 n=8+10)

name                           old alloc/op   new alloc/op   delta
WindowsInvalidFilenameValid-8     16.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)
WindowsInvalidFilenameNUL-8       19.0B ± 0%      3.0B ± 0%   -84.21%  (p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
WindowsInvalidFilenameValid-8      1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
WindowsInvalidFilenameNUL-8        2.00 ± 0%      1.00 ± 0%   -50.00%  (p=0.000 n=10+10)
@calmh
Copy link
Member

calmh commented Nov 21, 2022

This will have to wait because we're still targeting 1.18 until 1.20 comes out.

@imsodin
Copy link
Member

imsodin commented Nov 21, 2022

This will have to wait because we're still targeting 1.18 until 1.20 comes out.

Seems to be a go1.18 change - CI for that version came up green.

@calmh
Copy link
Member

calmh commented Nov 21, 2022

Oh, indeed. I had a feeling it was Go 1.19 and discounted the check because I had another feeling this was Windows-only code and not built... But I was wrong on all points :D

The Mac failure is a signing issue which I shall sort out.

@calmh calmh merged commit 663106e into syncthing:main Nov 21, 2022
@calmh calmh added this to the v1.22.2 milestone Nov 22, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Nov 22, 2022
* main: (23 commits)
  lib/fs: Optimize WindowsInvalidFilename (syncthing#8687)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing: Use main logger in generate subcommand (fixes syncthing#8682) (syncthing#8685)
  build: Update all dependencies (fixes syncthing#8679) (syncthing#8680)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Correctly set xattrs on temp files (fixes syncthing#8667) (syncthing#8670)
  gui: Automatically dismiss authentication reminder when in LDAP mode (fixes syncthing#8661) (syncthing#8663)
  lib/model: Correctly handle xattrs on directories (fixes syncthing#8657) (syncthing#8658)
  lib/protocol: Ignore inode time when xattr&ownership is ignored (fixes syncthing#8654) (syncthing#8655)
  lib/fs: Try to remove read only Windows files (fixes syncthing#3744) (syncthing#8650)
  gui: Add copy to clipboard, share by email, and share by SMS buttons to device IDs (fixes syncthing#2771, ref syncthing#3868) (syncthing#7984)
  gui, man, authors: Update docs, translations, and contributors
  build: Add GitHub actions build for Windows (syncthing#8627)
  gui: Fix connection type icon width (fixes syncthing#8592) (syncthing#8644)
  gui: Adjust connection type icon size scaling and alignment (syncthing#8645)
  docker: Use healthcheck endpoint (syncthing#8640)
  lib/connections: Use adaptive write size for rate limited connections (fixes syncthing#8630) (syncthing#8631)
  gui: Mark devices that haven't connected for a long time (fixes syncthing#7703) (syncthing#8530)
  gui: Fix rescan interval when add encrypted folder with watch for changes enabled (fixes syncthing#8570) (syncthing#8571)
  gui: Always show Out of Sync Items for remote devices (syncthing#8632)
  ...
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Nov 21, 2023
@syncthing syncthing locked and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants