Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006148OCamlOCaml standard librarypublic2013-08-29 20:582013-08-30 15:16
Reporterjohnwhitington 
Assigned Tofrisch 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version4.02.0+dev 
Summary0006148: Simple improvements to speed of Buffer module
DescriptionThe (tiny) attached patch to trunk uses unsafe functions in uncontroversial parts of the Buffer module. In all instances, the accesses are already checked by the logic of the Buffer module prior to the unsafe function being called.

I have avoided unsafe functions in complicated or infrequently called parts of Buffer, like the resize function.

Sample speedups in native code:
add_char heavy code: 12%
add_string heavy code: 19%
add_substring heavy code: 25%

Here's the add_string example:

let b = Buffer.create 30

let string_of_lexemes lexemes =
  Buffer.clear b;
  List.iter (fun l -> Buffer.add_string b l; Buffer.add_char b ' ') lexemes;
  Buffer.contents b

let _ =
  for x = 1 to 50_000_000 do
    ignore (string_of_lexemes ["a"; "bcd"; "efg"; "hij"; "klm"])
  done

The speedups in bytecode are similar.
TagsNo tags attached.
Attached Filesdiff file icon buffer-unsafe.diff [^] (477 bytes) 2013-08-29 20:58 [Show Content]
diff file icon buffer-unsafe-2.diff [^] (1,392 bytes) 2013-08-30 09:40 [Show Content]

- Relationships

-  Notes
(0010260)
ygrek (reporter)
2013-08-30 05:29

The patch seems to be reverse, and could you please use unified diff?
I.e. diff -urNw orig/ modified/
(0010261)
johnwhitington (reporter)
2013-08-30 09:40

See attached buffer-unsafe-2.diff
(0010264)
frisch (developer)
2013-08-30 15:16

Thanks! I've integrated your patch to the trunk (commit 14048).

As a side note, it's not 100% straightforward that the calls to blit in add_substring and add_string are safe. I'm thinking about possible integer overflow when computing new_position (leading to a negative value for new_position which does not trigger resizing). I think it is fine because of the limit on the maximum string length, but one needs to be careful (I've heard about a variant of the OCaml runtime developed by OCamlPro which gets rid of the limit on string lenths).

- Issue History
Date Modified Username Field Change
2013-08-29 20:58 johnwhitington New Issue
2013-08-29 20:58 johnwhitington File Added: buffer-unsafe.diff
2013-08-30 05:29 ygrek Note Added: 0010260
2013-08-30 09:40 johnwhitington Note Added: 0010261
2013-08-30 09:40 johnwhitington File Added: buffer-unsafe-2.diff
2013-08-30 15:16 frisch Note Added: 0010264
2013-08-30 15:16 frisch Status new => resolved
2013-08-30 15:16 frisch Fixed in Version => 4.02.0+dev
2013-08-30 15:16 frisch Resolution open => fixed
2013-08-30 15:16 frisch Assigned To => frisch


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker