Skip to content

perf: eliminate redundant syscalls in JSON serialization and filesystem operations #223

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.

Sign up for GitHub

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

Draft
codspeed-hq wants to merge 1 commit into
base: main
Choose a base branch
from
+66 −29
Draft

perf: eliminate redundant syscalls in JSON serialization and filesystem operations #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions sandbox-api/src/handler/base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
"github.com/gin-gonic/gin"
)

// jsonContentType is the Content-Type header value for JSON responses.
// Pre-allocated as a slice to allow direct header map assignment without allocation.
var jsonContentType = []string{"application/json; charset=utf-8"}

// BaseHandler provides common functionality for both MCP and API handlers
type BaseHandler struct {
// Add any common fields here
Expand All @@ -28,30 +32,45 @@ type SuccessResponse struct {
Message string `json:"message" example:"File created successfully" binding:"required"`
} // @name SuccessResponse

// writeJSONResponse serializes data using jsoniter and writes it directly to the
// response writer, bypassing Gin's c.JSON() which uses the slower encoding/json.
// The package-level `json` variable (defined in filesystem.go) is
// jsoniter.ConfigCompatibleWithStandardLibrary.
func writeJSONResponse(c *gin.Context, status int, data interface{}) {
buf, err := json.Marshal(data)
if err != nil {
c.Status(http.StatusInternalServerError)
return
}
c.Status(status)
c.Writer.Header()["Content-Type"] = jsonContentType
_, _ = c.Writer.Write(buf)
}

// SendError sends a standardized error response
func (h *BaseHandler) SendError(c *gin.Context, status int, err error) {
c.JSON(status, ErrorResponse{
writeJSONResponse(c, status, ErrorResponse{
Error: err.Error(),
})
}

// SendSuccess sends a standardized success response
func (h *BaseHandler) SendSuccess(c *gin.Context, message string) {
c.JSON(http.StatusOK, SuccessResponse{
writeJSONResponse(c, http.StatusOK, SuccessResponse{
Message: message,
})
}

func (h *BaseHandler) SendSuccessWithPath(c *gin.Context, path string, message string) {
c.JSON(http.StatusOK, SuccessResponse{
writeJSONResponse(c, http.StatusOK, SuccessResponse{
Path: path,
Message: message,
})
}

// SendJSON sends a JSON response with the given status code
func (h *BaseHandler) SendJSON(c *gin.Context, status int, data interface{}) {
c.JSON(status, data)
writeJSONResponse(c, status, data)
}

// GetPathParam gets a path parameter and returns an error if it's invalid
Expand All @@ -72,9 +91,11 @@ func (h *BaseHandler) GetQueryParam(c *gin.Context, param string, defaultValue s
return value
}

// BindJSON binds the request body to a struct and returns an error if it fails
// BindJSON reads the request body and deserializes it using jsoniter,
// bypassing Gin's ShouldBindJSON which uses the slower encoding/json.
func (h *BaseHandler) BindJSON(c *gin.Context, obj interface{}) error {
if err := c.ShouldBindJSON(obj); err != nil {
decoder := json.NewDecoder(c.Request.Body)
if err := decoder.Decode(obj); err != nil {
return fmt.Errorf("invalid request body: %w", err)
Comment on lines +97 to 99

@mendral-app mendral-app Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug (P2): Switching from ShouldBindJSON to raw json.NewDecoder().Decode() silently drops Gin's struct validation. Any request struct with binding:"required" tags (e.g., ProcessRequest.Command, DriveMountRequest.DriveName) will no longer have those constraints enforced, allowing invalid requests through.

Suggested change
Suggested change
decoder := json.NewDecoder(c.Request.Body)
if err := decoder.Decode(obj); err != nil {
return fmt.Errorf("invalid request body: %w", err)
func (h *BaseHandler) BindJSON(c *gin.Context, obj interface{}) error {
if err := json.NewDecoder(c.Request.Body).Decode(obj); err != nil {
return fmt.Errorf("invalid request body: %w", err)
}
if err := binding.Validator.ValidateStruct(obj); err != nil {
return fmt.Errorf("invalid request body: %w", err)
}
return nil
}
Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At sandbox-api/src/handler/base.go, line 97:

<issue>
Switching from `ShouldBindJSON` to raw `json.NewDecoder().Decode()` silently drops Gin's struct validation. Any request struct with `binding:"required"` tags (e.g., `ProcessRequest.Command`, `DriveMountRequest.DriveName`) will no longer have those constraints enforced, allowing invalid requests through.
</issue>

@ouillie ouillie Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's break these JSON-related changes into a separate PR. They seem completely unrelated to the Lstat-related changes.

@mendral-app mendral-app Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. The JSON serialization changes (switching to jsoniter, adding writeJSONResponse(), updating BindJSON() in base.go) and the Lstat optimizations (eliminating redundant syscalls in filesystem.go) are independent optimizations targeting different layers — HTTP serialization vs. filesystem I/O. They don't share any functional dependency.

Splitting them would make each PR easier to review, benchmark independently, and revert if needed.

}
return nil
Expand All @@ -87,7 +108,7 @@ type WelcomeResponse struct {
}

func (h *BaseHandler) HandleWelcome(c *gin.Context) {
c.JSON(http.StatusOK, WelcomeResponse{
writeJSONResponse(c, http.StatusOK, WelcomeResponse{
Message: "Welcome to your Blaxel Sandbox",
Documentation: "https://docs.blaxel.ai/Sandboxes/Overview",
Description: "This sandbox provides a full-featured environment for running code securely. Visit the documentation to learn how to manage processes, access the filesystem, and more.",
Expand Down
60 changes: 38 additions & 22 deletions sandbox-api/src/handler/filesystem/filesystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ func (fs *Filesystem) ReadFile(path string) (*FileWithContentByte, error) {
return nil, err
}

// Get owner and group
owner, group, err := fs.getFileOwnerAndGroup(absPath)
// Get owner and group from the already-obtained file info,
// avoiding a redundant os.Lstat call
owner, group, err := getOwnerAndGroupFromInfo(info)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -386,20 +387,26 @@ func (fs *Filesystem) ListDirectory(path string) (*Directory, error) {
for _, entry := range entries {
// Use displayPath for the entry paths too
entryPath := filepath.Join(displayPath, entry.Name())
absEntryPath := filepath.Join(absPath, entry.Name())

// Use os.Lstat to get info about the symlink itself, not its target
// This prevents errors when symlinks point to non-existent targets
info, err := os.Lstat(absEntryPath)
if err != nil {
return nil, err
}

if info.IsDir() {
// Use entry.IsDir() from DirEntry which uses the file type cached
// by os.ReadDir (from getdents64 on Linux), avoiding an os.Lstat syscall
// for directory entries entirely.
if entry.IsDir() {
dir.AddSubdirectory(&Subdirectory{Path: entryPath, Name: entry.Name()})
} else {
// It's a file or symlink
owner, group, err := fs.getFileOwnerAndGroup(absEntryPath)
// For files and symlinks, we need stat info for metadata
absEntryPath := filepath.Join(absPath, entry.Name())

// Use os.Lstat to get info about the symlink itself, not its target

@ouillie ouillie Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-use fs.getFileOwnerAndGroup here

@ouillie ouillie Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, nevermind, I didn't realize that info is used later on as well.

// This prevents errors when symlinks point to non-existent targets
info, err := os.Lstat(absEntryPath)
if err != nil {
return nil, err
}

// Extract owner and group from the already-obtained FileInfo,
// avoiding a redundant os.Lstat call in getFileOwnerAndGroup
owner, group, err := getOwnerAndGroupFromInfo(info)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -508,14 +515,10 @@ func (fs *Filesystem) MoveFile(src, dst string) error {
return os.Rename(srcAbs, dstAbs)
}

// getFileOwnerAndGroup returns the owner and group of a file
func (fs *Filesystem) getFileOwnerAndGroup(path string) (string, string, error) {
// Use Lstat to get info about the symlink itself, not its target
info, err := os.Lstat(path)
if err != nil {
return "", "", err
}

// getOwnerAndGroupFromInfo extracts the owner and group from an already-obtained
// os.FileInfo, avoiding a redundant os.Lstat syscall. This is used by callers
// that have already stat'd the file (ListDirectory, ReadFile, GetFileInfo).
func getOwnerAndGroupFromInfo(info os.FileInfo) (string, string, error) {
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return "", "", errors.New("failed to get file stat")
Expand All @@ -541,6 +544,17 @@ func (fs *Filesystem) getFileOwnerAndGroup(path string) (string, string, error)
return ownerName, groupName, nil
}

// getFileOwnerAndGroup returns the owner and group of a file
func (fs *Filesystem) getFileOwnerAndGroup(path string) (string, string, error) {
// Use Lstat to get info about the symlink itself, not its target
info, err := os.Lstat(path)
if err != nil {
return "", "", err
}

return getOwnerAndGroupFromInfo(info)
}

// GetFileInfo returns file information without reading its content
func (fs *Filesystem) GetFileInfo(path string) (*FileByte, error) {
absPath, err := fs.GetAbsolutePath(path)
Expand All @@ -557,7 +571,9 @@ func (fs *Filesystem) GetFileInfo(path string) (*FileByte, error) {
return nil, errors.New("path points to a directory, not a file")
}

owner, group, err := fs.getFileOwnerAndGroup(absPath)
// Get owner and group from the already-obtained file info,
// avoiding a redundant os.Lstat call
owner, group, err := getOwnerAndGroupFromInfo(info)
if err != nil {
return nil, err
}
Expand Down